diff options
author | glozow <gloriajzhao@gmail.com> | 2022-08-03 09:43:59 +0100 |
---|---|---|
committer | glozow <gloriajzhao@gmail.com> | 2022-08-03 09:47:01 +0100 |
commit | f6fdedf850d10d877316871aacfd5b6656178e70 (patch) | |
tree | 68a34f2290d787115610163fb28506e3493f8013 /src | |
parent | de3c46c93818c6e4000175bcdbf7dd332f54768d (diff) | |
parent | ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 (diff) |
Merge bitcoin/bitcoin#25648: refactor: Remove all policy globals
ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 sort after scripted-diff (MacroFake)
fac812ca835e0d843aba1d4db0e49d183018a29e scripted-diff: Move mempool_args to src/node (MacroFake)
66664384a6fec39ecb4d8d06db66a4f193a06e33 Remove ::g_max_datacarrier_bytes global (MacroFake)
fad0b4fab849eb5f1f0aa54ebc290f85a473ec91 Pass datacarrier setting into IsStandard (MacroFake)
fa2a6b8516b24d7e9ca11926a49cf2b07f661e81 Combine datacarrier globals into one (MacroFake)
fa477d32eefcc3dd2f06b452066290d9936d8c5d Remove ::GetVirtualTransactionSize() alias (MacroFake)
fa2f6c1a611dffe5a3f63fe1b453f1dd420371b1 Remove ::fIsBareMultisigStd global (MacroFake)
fadc14e4f514e7167723285e0ac3d4a7149bbee6 Remove ::dustRelayFee (MacroFake)
fa8a7f01fe1b6db98097021276ed5d929faadbec Remove ::IsStandardTx(tx, reason) alias (MacroFake)
fa7a9114e59b81b50584311a4ab2b3e9a8d956bd test: Remove unused cs_main (MacroFake)
fa9cba7afb73c01bd2c8fefd662dfc80dd98c5e8 Remove ::incrementalRelayFee and ::minRelayTxFee globals (MacroFake)
fa148602e67fe035b1b21eff6c0b656919ac2d45 Remove ::fRequireStandard global (MacroFake)
fa468bdfb62dec286cb977db78d3e47b64dafeba Return optional error from ApplyArgsManOptions (MacroFake)
Pull request description:
This change is good because:
* It moves module-specific init-logic out of the bloated init.cpp
* It removes a global from validation.cpp and places it into the data structure that needs it (mempool)
ACKs for top commit:
glozow:
re ACK ddddd69
ryanofsky:
Code review ACK ddddd6913b1bdee1cad89a32d363306ea1f7b8d7
ariard:
Light Code Review ACK ddddd69
Tree-SHA512: 9de2ce601cfcaa4dfd7d1c92270568895ce8702ccdffb59829fbe9618eab0fd88d738afef33ed66988c66861115e0340e881056bfb71e2aed4af2440bd37eb1e
Diffstat (limited to 'src')
34 files changed, 262 insertions, 223 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 23bc180095..22390ef2bf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -139,7 +139,6 @@ BITCOIN_CORE_H = \ compat/cpuid.h \ compat/endian.h \ compressor.h \ - node/connection_types.h \ consensus/consensus.h \ consensus/tx_check.h \ consensus/tx_verify.h \ @@ -149,7 +148,6 @@ BITCOIN_CORE_H = \ dbwrapper.h \ deploymentinfo.h \ deploymentstatus.h \ - node/eviction.h \ external_signer.h \ flatfile.h \ fs.h \ @@ -184,7 +182,6 @@ BITCOIN_CORE_H = \ logging.h \ logging/timer.h \ mapport.h \ - mempool_args.h \ memusage.h \ merkleblock.h \ net.h \ @@ -199,13 +196,16 @@ BITCOIN_CORE_H = \ node/caches.h \ node/chainstate.h \ node/coin.h \ + node/connection_types.h \ node/context.h \ + node/eviction.h \ + node/interface_ui.h \ + node/mempool_args.h \ node/mempool_persist_args.h \ node/miner.h \ node/minisketchwrapper.h \ node/psbt.h \ node/transaction.h \ - node/interface_ui.h \ node/utxo_snapshot.h \ noui.h \ outputtype.h \ @@ -372,10 +372,9 @@ libbitcoin_node_a_SOURCES = \ kernel/context.cpp \ kernel/mempool_persist.cpp \ mapport.cpp \ - mempool_args.cpp \ net.cpp \ - netgroup.cpp \ net_processing.cpp \ + netgroup.cpp \ node/blockstorage.cpp \ node/caches.cpp \ node/chainstate.cpp \ @@ -383,13 +382,14 @@ libbitcoin_node_a_SOURCES = \ node/connection_types.cpp \ node/context.cpp \ node/eviction.cpp \ + node/interface_ui.cpp \ node/interfaces.cpp \ + node/mempool_args.cpp \ node/mempool_persist_args.cpp \ node/miner.cpp \ node/minisketchwrapper.cpp \ node/psbt.cpp \ node/transaction.cpp \ - node/interface_ui.cpp \ noui.cpp \ policy/fees.cpp \ policy/fees_args.cpp \ @@ -402,8 +402,8 @@ libbitcoin_node_a_SOURCES = \ rpc/fees.cpp \ rpc/mempool.cpp \ rpc/mining.cpp \ - rpc/node.cpp \ rpc/net.cpp \ + rpc/node.cpp \ rpc/output_script.cpp \ rpc/rawtransaction.cpp \ rpc/server.cpp \ diff --git a/src/init.cpp b/src/init.cpp index a94bbe6460..e3d13ad972 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -31,7 +31,6 @@ #include <interfaces/init.h> #include <interfaces/node.h> #include <mapport.h> -#include <mempool_args.h> #include <net.h> #include <net_permissions.h> #include <net_processing.h> @@ -42,6 +41,7 @@ #include <node/chainstate.h> #include <node/context.h> #include <node/interface_ui.h> +#include <node/mempool_args.h> #include <node/mempool_persist_args.h> #include <node/miner.h> #include <policy/feerate.h> @@ -935,16 +935,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb LogPrintf("Warning: nMinimumChainWork set below default value of %s\n", chainparams.GetConsensus().nMinimumChainWork.GetHex()); } - // incremental relay fee sets the minimum feerate increase necessary for BIP 125 replacement in the mempool - // and the amount the mempool min fee increases above the feerate of txs evicted due to mempool limiting. - if (args.IsArgSet("-incrementalrelayfee")) { - if (std::optional<CAmount> inc_relay_fee = ParseMoney(args.GetArg("-incrementalrelayfee", ""))) { - ::incrementalRelayFee = CFeeRate{inc_relay_fee.value()}; - } else { - return InitError(AmountErrMsg("incrementalrelayfee", args.GetArg("-incrementalrelayfee", ""))); - } - } - // block pruning; get the amount of disk space (in MiB) to allot for block & undo files int64_t nPruneArg = args.GetIntArg("-prune", 0); if (nPruneArg < 0) { @@ -973,19 +963,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb return InitError(Untranslated("peertimeout must be a positive integer.")); } - if (args.IsArgSet("-minrelaytxfee")) { - if (std::optional<CAmount> min_relay_fee = ParseMoney(args.GetArg("-minrelaytxfee", ""))) { - // High fee check is done afterward in CWallet::Create() - ::minRelayTxFee = CFeeRate{min_relay_fee.value()}; - } else { - return InitError(AmountErrMsg("minrelaytxfee", args.GetArg("-minrelaytxfee", ""))); - } - } else if (incrementalRelayFee > ::minRelayTxFee) { - // Allow only setting incrementalRelayFee to control both - ::minRelayTxFee = incrementalRelayFee; - LogPrintf("Increasing minrelaytxfee to %s to match incrementalrelayfee\n",::minRelayTxFee.ToString()); - } - // Sanity check argument for min fee for including tx in block // TODO: Harmonize which arguments need sanity checking and where that happens if (args.IsArgSet("-blockmintxfee")) { @@ -994,28 +971,10 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb } } - // Feerate used to define dust. Shouldn't be changed lightly as old - // implementations may inadvertently create non-standard transactions - if (args.IsArgSet("-dustrelayfee")) { - if (std::optional<CAmount> parsed = ParseMoney(args.GetArg("-dustrelayfee", ""))) { - dustRelayFee = CFeeRate{parsed.value()}; - } else { - return InitError(AmountErrMsg("dustrelayfee", args.GetArg("-dustrelayfee", ""))); - } - } - - fRequireStandard = !args.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard()); - if (!chainparams.IsTestChain() && !fRequireStandard) { - return InitError(strprintf(Untranslated("acceptnonstdtxn is not currently supported for %s chain"), chainparams.NetworkIDString())); - } nBytesPerSigOp = args.GetIntArg("-bytespersigop", nBytesPerSigOp); if (!g_wallet_init_interface.ParameterInteraction()) return false; - fIsBareMultisigStd = args.GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG); - fAcceptDatacarrier = args.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER); - nMaxDatacarrierBytes = args.GetIntArg("-datacarriersize", nMaxDatacarrierBytes); - // Option to startup with mocktime set (used for regression testing): SetMockTime(args.GetIntArg("-mocktime", 0)); // SetMockTime(0) is a no-op @@ -1418,7 +1377,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) .estimator = node.fee_estimator.get(), .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, }; - ApplyArgsManOptions(args, mempool_opts); + if (const auto err{ApplyArgsManOptions(args, chainparams, mempool_opts)}) { + return InitError(*err); + } mempool_opts.check_ratio = std::clamp<int>(mempool_opts.check_ratio, 0, 1'000'000); int64_t descendant_limit_bytes = mempool_opts.limits.descendant_size_vbytes * 40; diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 07953b443b..dad6f14c39 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -6,8 +6,13 @@ #include <kernel/mempool_limits.h> +#include <policy/feerate.h> +#include <policy/policy.h> +#include <script/standard.h> + #include <chrono> #include <cstdint> +#include <optional> class CBlockPolicyEstimator; @@ -33,6 +38,20 @@ struct MemPoolOptions { int check_ratio{0}; int64_t max_size_bytes{DEFAULT_MAX_MEMPOOL_SIZE_MB * 1'000'000}; std::chrono::seconds expiry{std::chrono::hours{DEFAULT_MEMPOOL_EXPIRY_HOURS}}; + CFeeRate incremental_relay_feerate{DEFAULT_INCREMENTAL_RELAY_FEE}; + /** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */ + CFeeRate min_relay_feerate{DEFAULT_MIN_RELAY_TX_FEE}; + CFeeRate dust_relay_feerate{DUST_RELAY_TX_FEE}; + /** + * A data carrying output is an unspendable output containing data. The script + * type is designated as TxoutType::NULL_DATA. + * + * Maximum size of TxoutType::NULL_DATA scripts that this node considers standard. + * If nullopt, any size is nonstandard. + */ + std::optional<unsigned> max_datacarrier_bytes{DEFAULT_ACCEPT_DATACARRIER ? std::optional{MAX_OP_RETURN_RELAY} : std::nullopt}; + bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG}; + bool require_standard{true}; bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF}; MemPoolLimits limits{}; }; diff --git a/src/mempool_args.cpp b/src/mempool_args.cpp deleted file mode 100644 index 77caa127e9..0000000000 --- a/src/mempool_args.cpp +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) 2022 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <mempool_args.h> - -#include <kernel/mempool_limits.h> -#include <kernel/mempool_options.h> - -#include <util/system.h> - -using kernel::MemPoolLimits; -using kernel::MemPoolOptions; - -namespace { -void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolLimits& mempool_limits) -{ - mempool_limits.ancestor_count = argsman.GetIntArg("-limitancestorcount", mempool_limits.ancestor_count); - - if (auto vkb = argsman.GetIntArg("-limitancestorsize")) mempool_limits.ancestor_size_vbytes = *vkb * 1'000; - - mempool_limits.descendant_count = argsman.GetIntArg("-limitdescendantcount", mempool_limits.descendant_count); - - if (auto vkb = argsman.GetIntArg("-limitdescendantsize")) mempool_limits.descendant_size_vbytes = *vkb * 1'000; -} -} - -void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolOptions& mempool_opts) -{ - mempool_opts.check_ratio = argsman.GetIntArg("-checkmempool", mempool_opts.check_ratio); - - if (auto mb = argsman.GetIntArg("-maxmempool")) mempool_opts.max_size_bytes = *mb * 1'000'000; - - if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours}; - - mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf); - - ApplyArgsManOptions(argsman, mempool_opts.limits); -} diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0e10fa5f9d..2e11390541 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4759,8 +4759,8 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi } if (current_time > peer.m_next_send_feefilter) { CAmount filterToSend = g_filter_rounder.round(currentFilter); - // We always have a fee filter of at least minRelayTxFee - filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); + // We always have a fee filter of at least the min relay fee + filterToSend = std::max(filterToSend, m_mempool.m_min_relay_feerate.GetFeePerK()); if (filterToSend != peer.m_fee_filter_sent) { m_connman.PushMessage(&pto, CNetMsgMaker(pto.GetCommonVersion()).Make(NetMsgType::FEEFILTER, filterToSend)); peer.m_fee_filter_sent = filterToSend; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 699dca0a73..2c845d0127 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -301,7 +301,11 @@ public: } } bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); } - CFeeRate getDustRelayFee() override { return ::dustRelayFee; } + CFeeRate getDustRelayFee() override + { + if (!m_context->mempool) return CFeeRate{DUST_RELAY_TX_FEE}; + return m_context->mempool->m_dust_relay_feerate; + } UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override { JSONRPCRequest req; @@ -676,9 +680,21 @@ public: if (!m_node.mempool) return {}; return m_node.mempool->GetMinFee(); } - CFeeRate relayMinFee() override { return ::minRelayTxFee; } - CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; } - CFeeRate relayDustFee() override { return ::dustRelayFee; } + CFeeRate relayMinFee() override + { + if (!m_node.mempool) return CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}; + return m_node.mempool->m_min_relay_feerate; + } + CFeeRate relayIncrementalFee() override + { + if (!m_node.mempool) return CFeeRate{DEFAULT_INCREMENTAL_RELAY_FEE}; + return m_node.mempool->m_incremental_relay_feerate; + } + CFeeRate relayDustFee() override + { + if (!m_node.mempool) return CFeeRate{DUST_RELAY_TX_FEE}; + return m_node.mempool->m_dust_relay_feerate; + } bool havePruned() override { LOCK(::cs_main); diff --git a/src/node/mempool_args.cpp b/src/node/mempool_args.cpp new file mode 100644 index 0000000000..60993f1d8d --- /dev/null +++ b/src/node/mempool_args.cpp @@ -0,0 +1,99 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <node/mempool_args.h> + +#include <kernel/mempool_limits.h> +#include <kernel/mempool_options.h> + +#include <chainparams.h> +#include <consensus/amount.h> +#include <logging.h> +#include <policy/feerate.h> +#include <policy/policy.h> +#include <tinyformat.h> +#include <util/error.h> +#include <util/moneystr.h> +#include <util/system.h> +#include <util/translation.h> + +#include <chrono> +#include <memory> + +using kernel::MemPoolLimits; +using kernel::MemPoolOptions; + +namespace { +void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolLimits& mempool_limits) +{ + mempool_limits.ancestor_count = argsman.GetIntArg("-limitancestorcount", mempool_limits.ancestor_count); + + if (auto vkb = argsman.GetIntArg("-limitancestorsize")) mempool_limits.ancestor_size_vbytes = *vkb * 1'000; + + mempool_limits.descendant_count = argsman.GetIntArg("-limitdescendantcount", mempool_limits.descendant_count); + + if (auto vkb = argsman.GetIntArg("-limitdescendantsize")) mempool_limits.descendant_size_vbytes = *vkb * 1'000; +} +} + +std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& argsman, const CChainParams& chainparams, MemPoolOptions& mempool_opts) +{ + mempool_opts.check_ratio = argsman.GetIntArg("-checkmempool", mempool_opts.check_ratio); + + if (auto mb = argsman.GetIntArg("-maxmempool")) mempool_opts.max_size_bytes = *mb * 1'000'000; + + if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours}; + + // incremental relay fee sets the minimum feerate increase necessary for BIP 125 replacement in the mempool + // and the amount the mempool min fee increases above the feerate of txs evicted due to mempool limiting. + if (argsman.IsArgSet("-incrementalrelayfee")) { + if (std::optional<CAmount> inc_relay_fee = ParseMoney(argsman.GetArg("-incrementalrelayfee", ""))) { + mempool_opts.incremental_relay_feerate = CFeeRate{inc_relay_fee.value()}; + } else { + return AmountErrMsg("incrementalrelayfee", argsman.GetArg("-incrementalrelayfee", "")); + } + } + + if (argsman.IsArgSet("-minrelaytxfee")) { + if (std::optional<CAmount> min_relay_feerate = ParseMoney(argsman.GetArg("-minrelaytxfee", ""))) { + // High fee check is done afterward in CWallet::Create() + mempool_opts.min_relay_feerate = CFeeRate{min_relay_feerate.value()}; + } else { + return AmountErrMsg("minrelaytxfee", argsman.GetArg("-minrelaytxfee", "")); + } + } else if (mempool_opts.incremental_relay_feerate > mempool_opts.min_relay_feerate) { + // Allow only setting incremental fee to control both + mempool_opts.min_relay_feerate = mempool_opts.incremental_relay_feerate; + LogPrintf("Increasing minrelaytxfee to %s to match incrementalrelayfee\n", mempool_opts.min_relay_feerate.ToString()); + } + + // Feerate used to define dust. Shouldn't be changed lightly as old + // implementations may inadvertently create non-standard transactions + if (argsman.IsArgSet("-dustrelayfee")) { + if (std::optional<CAmount> parsed = ParseMoney(argsman.GetArg("-dustrelayfee", ""))) { + mempool_opts.dust_relay_feerate = CFeeRate{parsed.value()}; + } else { + return AmountErrMsg("dustrelayfee", argsman.GetArg("-dustrelayfee", "")); + } + } + + mempool_opts.permit_bare_multisig = argsman.GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG); + + if (argsman.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) { + mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY); + } else { + mempool_opts.max_datacarrier_bytes = std::nullopt; + } + + mempool_opts.require_standard = !argsman.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard()); + if (!chainparams.IsTestChain() && !mempool_opts.require_standard) { + return strprintf(Untranslated("acceptnonstdtxn is not currently supported for %s chain"), chainparams.NetworkIDString()); + } + + mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf); + + ApplyArgsManOptions(argsman, mempool_opts.limits); + + return std::nullopt; +} diff --git a/src/mempool_args.h b/src/node/mempool_args.h index 9a4abe6618..52d8b4f265 100644 --- a/src/mempool_args.h +++ b/src/node/mempool_args.h @@ -2,21 +2,26 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_MEMPOOL_ARGS_H -#define BITCOIN_MEMPOOL_ARGS_H +#ifndef BITCOIN_NODE_MEMPOOL_ARGS_H +#define BITCOIN_NODE_MEMPOOL_ARGS_H + +#include <optional> class ArgsManager; +class CChainParams; +struct bilingual_str; namespace kernel { struct MemPoolOptions; }; /** * Overlay the options set in \p argsman on top of corresponding members in \p mempool_opts. + * Returns an error if one was encountered. * * @param[in] argsman The ArgsManager in which to check set options. * @param[in,out] mempool_opts The MemPoolOptions to modify according to \p argsman. */ -void ApplyArgsManOptions(const ArgsManager& argsman, kernel::MemPoolOptions& mempool_opts); +[[nodiscard]] std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& argsman, const CChainParams& chainparams, kernel::MemPoolOptions& mempool_opts); -#endif // BITCOIN_MEMPOOL_ARGS_H +#endif // BITCOIN_NODE_MEMPOOL_ARGS_H diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 5a932f435d..57162cd679 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -137,7 +137,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) if (success) { CTransaction ctx = CTransaction(mtx); - size_t size = GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS)); + size_t size(GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS), ::nBytesPerSigOp)); result.estimated_vsize = size; // Estimate fee rate CFeeRate feerate(fee, size); diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index f6452266b7..5086542865 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -67,7 +67,7 @@ bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn) return (txout.nValue < GetDustThreshold(txout, dustRelayFeeIn)); } -bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType) +bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_datacarrier_bytes, TxoutType& whichType) { std::vector<std::vector<unsigned char> > vSolutions; whichType = Solver(scriptPubKey, vSolutions); @@ -82,15 +82,16 @@ bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType) return false; if (m < 1 || m > n) return false; - } else if (whichType == TxoutType::NULL_DATA && - (!fAcceptDatacarrier || scriptPubKey.size() > nMaxDatacarrierBytes)) { - return false; + } else if (whichType == TxoutType::NULL_DATA) { + if (!max_datacarrier_bytes || scriptPubKey.size() > *max_datacarrier_bytes) { + return false; + } } return true; } -bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason) +bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason) { if (tx.nVersion > TX_MAX_STANDARD_VERSION || tx.nVersion < 1) { reason = "version"; @@ -130,7 +131,7 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR unsigned int nDataOut = 0; TxoutType whichType; for (const CTxOut& txout : tx.vout) { - if (!::IsStandard(txout.scriptPubKey, whichType)) { + if (!::IsStandard(txout.scriptPubKey, max_datacarrier_bytes, whichType)) { reason = "scriptpubkey"; return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index cd98a601a3..3d2660b081 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -47,8 +47,8 @@ static constexpr unsigned int MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE{80}; static constexpr unsigned int MAX_STANDARD_P2WSH_SCRIPT_SIZE{3600}; /** The maximum size of a standard ScriptSig */ static constexpr unsigned int MAX_STANDARD_SCRIPTSIG_SIZE{1650}; -/** Min feerate for defining dust. Historically this has been based on the - * minRelayTxFee, however changing the dust limit changes which transactions are +/** Min feerate for defining dust. + * Changing the dust limit changes which transactions are * standard and should be done with care and ideally rarely. It makes sense to * only increase the dust limit after prior releases were already not creating * outputs below the new threshold */ @@ -105,7 +105,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee); bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee); -bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType); +bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_datacarrier_bytes, TxoutType& whichType); // Changing the default transaction version requires a two step process: first @@ -117,7 +117,7 @@ static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{2}; * Check for standard transaction types * @return True if all outputs (scriptPubKeys) use only standard transaction forms */ -bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason); +bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason); /** * Check for standard transaction types * @param[in] mapInputs Map of previous transactions that have outputs we're spending diff --git a/src/policy/settings.cpp b/src/policy/settings.cpp index 0b67d274ce..39e00f1111 100644 --- a/src/policy/settings.cpp +++ b/src/policy/settings.cpp @@ -5,11 +5,6 @@ #include <policy/settings.h> -#include <policy/feerate.h> #include <policy/policy.h> -bool fIsBareMultisigStd = DEFAULT_PERMIT_BAREMULTISIG; -CFeeRate incrementalRelayFee = CFeeRate(DEFAULT_INCREMENTAL_RELAY_FEE); -CFeeRate dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); -CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); unsigned int nBytesPerSigOp = DEFAULT_BYTES_PER_SIGOP; diff --git a/src/policy/settings.h b/src/policy/settings.h index 2311d01fe8..f0d6f779ae 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -6,35 +6,6 @@ #ifndef BITCOIN_POLICY_SETTINGS_H #define BITCOIN_POLICY_SETTINGS_H -#include <policy/feerate.h> -#include <policy/policy.h> - -#include <cstdint> -#include <string> - -class CTransaction; - -// Policy settings which are configurable at runtime. -extern CFeeRate incrementalRelayFee; -extern CFeeRate dustRelayFee; -/** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */ -extern CFeeRate minRelayTxFee; extern unsigned int nBytesPerSigOp; -extern bool fIsBareMultisigStd; - -static inline bool IsStandardTx(const CTransaction& tx, std::string& reason) -{ - return IsStandardTx(tx, ::fIsBareMultisigStd, ::dustRelayFee, reason); -} - -static inline int64_t GetVirtualTransactionSize(int64_t weight, int64_t sigop_cost) -{ - return GetVirtualTransactionSize(weight, sigop_cost, ::nBytesPerSigOp); -} - -static inline int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t sigop_cost) -{ - return GetVirtualTransactionSize(tx, sigop_cost, ::nBytesPerSigOp); -} #endif // BITCOIN_POLICY_SETTINGS_H diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index 41f386d443..aa047bdea8 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -6,7 +6,6 @@ #include <core_io.h> #include <policy/feerate.h> #include <policy/fees.h> -#include <policy/settings.h> #include <rpc/protocol.h> #include <rpc/request.h> #include <rpc/server.h> @@ -88,7 +87,7 @@ static RPCHelpMan estimatesmartfee() CFeeRate feeRate{fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative)}; if (feeRate != CFeeRate(0)) { CFeeRate min_mempool_feerate{mempool.GetMinFee()}; - CFeeRate min_relay_feerate{::minRelayTxFee}; + CFeeRate min_relay_feerate{mempool.m_min_relay_feerate}; feeRate = std::max({feeRate, min_mempool_feerate, min_relay_feerate}); result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK())); } else { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 0ae10b6c39..02b51ce0a0 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -666,9 +666,9 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); ret.pushKV("total_fee", ValueFromAmount(pool.GetTotalFee())); ret.pushKV("maxmempool", pool.m_max_size_bytes); - ret.pushKV("mempoolminfee", ValueFromAmount(std::max(pool.GetMinFee(), ::minRelayTxFee).GetFeePerK())); - ret.pushKV("minrelaytxfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())); - ret.pushKV("incrementalrelayfee", ValueFromAmount(::incrementalRelayFee.GetFeePerK())); + ret.pushKV("mempoolminfee", ValueFromAmount(std::max(pool.GetMinFee(), pool.m_min_relay_feerate).GetFeePerK())); + ret.pushKV("minrelaytxfee", ValueFromAmount(pool.m_min_relay_feerate.GetFeePerK())); + ret.pushKV("incrementalrelayfee", ValueFromAmount(pool.m_incremental_relay_feerate.GetFeePerK())); ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()}); ret.pushKV("fullrbf", pool.m_full_rbf); return ret; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 059be61bb4..0ee905a77a 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -645,8 +645,11 @@ static RPCHelpMan getnetworkinfo() obj.pushKV("connections_out", node.connman->GetNodeCount(ConnectionDirection::Out)); } obj.pushKV("networks", GetNetworksInfo()); - obj.pushKV("relayfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())); - obj.pushKV("incrementalfee", ValueFromAmount(::incrementalRelayFee.GetFeePerK())); + if (node.mempool) { + // Those fields can be deprecated, to be replaced by the getmempoolinfo fields + obj.pushKV("relayfee", ValueFromAmount(node.mempool->m_min_relay_feerate.GetFeePerK())); + obj.pushKV("incrementalfee", ValueFromAmount(node.mempool->m_incremental_relay_feerate.GetFeePerK())); + } UniValue localAddresses(UniValue::VARR); { LOCK(g_maplocalhost_mutex); diff --git a/src/script/standard.cpp b/src/script/standard.cpp index b3f6a1b669..6101738061 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -16,9 +16,6 @@ typedef std::vector<unsigned char> valtype; -bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; -unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; - CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in)) {} CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast<uint160>(in)) {} diff --git a/src/script/standard.h b/src/script/standard.h index 448fdff010..1e6769782a 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -33,21 +33,12 @@ public: }; /** - * Default setting for nMaxDatacarrierBytes. 80 bytes of data, +1 for OP_RETURN, + * Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN, * +2 for the pushdata opcodes. */ static const unsigned int MAX_OP_RETURN_RELAY = 83; /** - * A data carrying output is an unspendable output containing data. The script - * type is designated as TxoutType::NULL_DATA. - */ -extern bool fAcceptDatacarrier; - -/** Maximum size of TxoutType::NULL_DATA scripts that this node considers standard. */ -extern unsigned nMaxDatacarrierBytes; - -/** * Mandatory script verification flags that all new blocks must comply with for * them to be valid. (but old blocks may not comply with) Currently just P2SH, * but in the future other flags may be added. diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 72574612a2..c52fca5fe8 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -87,9 +87,6 @@ FUZZ_TARGET_INIT(integer, initialize_integer) } (void)GetSizeOfCompactSize(u64); (void)GetSpecialScriptSize(u32); - if (!MultiplicationOverflow(i64, static_cast<int64_t>(::nBytesPerSigOp)) && !AdditionOverflow(i64 * ::nBytesPerSigOp, static_cast<int64_t>(4))) { - (void)GetVirtualTransactionSize(i64, i64); - } if (!MultiplicationOverflow(i64, static_cast<int64_t>(u32)) && !AdditionOverflow(i64, static_cast<int64_t>(4)) && !AdditionOverflow(i64 * u32, static_cast<int64_t>(4))) { (void)GetVirtualTransactionSize(i64, i64, u32); } diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index bfea9778f4..6d2d2e2bc5 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -157,12 +157,12 @@ FUZZ_TARGET_INIT(key, initialize_key) assert(fillable_signing_provider_pub.HaveKey(pubkey.GetID())); TxoutType which_type_tx_pubkey; - const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, which_type_tx_pubkey); + const bool is_standard_tx_pubkey = IsStandard(tx_pubkey_script, std::nullopt, which_type_tx_pubkey); assert(is_standard_tx_pubkey); assert(which_type_tx_pubkey == TxoutType::PUBKEY); TxoutType which_type_tx_multisig; - const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, which_type_tx_multisig); + const bool is_standard_tx_multisig = IsStandard(tx_multisig_script, std::nullopt, which_type_tx_multisig); assert(is_standard_tx_multisig); assert(which_type_tx_multisig == TxoutType::MULTISIG); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 4801635791..1a06ae886e 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <mempool_args.h> +#include <node/mempool_args.h> #include <policy/rbf.h> #include <primitives/transaction.h> #include <sync.h> diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index fdcd0da37d..69a4b782aa 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -55,7 +55,7 @@ FUZZ_TARGET_INIT(script, initialize_script) } TxoutType which_type; - bool is_standard_ret = IsStandard(script, which_type); + bool is_standard_ret = IsStandard(script, std::nullopt, which_type); if (!is_standard_ret) { assert(which_type == TxoutType::NONSTANDARD || which_type == TxoutType::NULL_DATA || diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 273aa0dc5c..7fa4523800 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -69,8 +69,8 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE}; std::string reason; - const bool is_standard_with_permit_bare_multisig = IsStandardTx(tx, /* permit_bare_multisig= */ true, dust_relay_fee, reason); - const bool is_standard_without_permit_bare_multisig = IsStandardTx(tx, /* permit_bare_multisig= */ false, dust_relay_fee, reason); + const bool is_standard_with_permit_bare_multisig = IsStandardTx(tx, std::nullopt, /* permit_bare_multisig= */ true, dust_relay_fee, reason); + const bool is_standard_without_permit_bare_multisig = IsStandardTx(tx, std::nullopt, /* permit_bare_multisig= */ false, dust_relay_fee, reason); if (is_standard_without_permit_bare_multisig) { assert(is_standard_with_permit_bare_multisig); } @@ -92,7 +92,6 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) (void)GetTransactionWeight(tx); (void)GetVirtualTransactionSize(tx); (void)IsFinalTx(tx, /* nBlockHeight= */ 1024, /* nBlockTime= */ 1024); - (void)IsStandardTx(tx, reason); (void)RecursiveDynamicUsage(tx); (void)SignalsOptInRBF(tx); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index cfb112879a..3191367870 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -3,8 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/validation.h> -#include <mempool_args.h> #include <node/context.h> +#include <node/mempool_args.h> #include <node/miner.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> @@ -116,7 +116,7 @@ void MockTime(FuzzedDataProvider& fuzzed_data_provider, const CChainState& chain SetMockTime(time); } -CTxMemPool MakeMempool(const NodeContext& node) +CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeContext& node) { // Take the default options for tests... CTxMemPool::Options mempool_opts{MemPoolOptionsForTest(node)}; @@ -124,6 +124,7 @@ CTxMemPool MakeMempool(const NodeContext& node) // ...override specific options for this specific fuzz suite mempool_opts.estimator = nullptr; mempool_opts.check_ratio = 1; + mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool(); // ...and construct a CTxMemPool from it return CTxMemPool{mempool_opts}; @@ -150,7 +151,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) constexpr CAmount SUPPLY_TOTAL{COINBASE_MATURITY * 50 * COIN}; SetMempoolConstraints(*node.args, fuzzed_data_provider); - CTxMemPool tx_pool_{MakeMempool(node)}; + CTxMemPool tx_pool_{MakeMempool(fuzzed_data_provider, node)}; MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_); chainstate.SetMempool(&tx_pool); @@ -237,7 +238,6 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) auto txr = std::make_shared<TransactionsDelta>(removed, added); RegisterSharedValidationInterface(txr); const bool bypass_limits = fuzzed_data_provider.ConsumeBool(); - ::fRequireStandard = fuzzed_data_provider.ConsumeBool(); // Make sure ProcessNewPackage on one transaction works. // The result is not guaranteed to be the same as what is returned by ATMP. @@ -325,7 +325,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) } SetMempoolConstraints(*node.args, fuzzed_data_provider); - CTxMemPool tx_pool_{MakeMempool(node)}; + CTxMemPool tx_pool_{MakeMempool(fuzzed_data_provider, node)}; MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_); LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) @@ -348,7 +348,6 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) const auto tx = MakeTransactionRef(mut_tx); const bool bypass_limits = fuzzed_data_provider.ConsumeBool(); - ::fRequireStandard = fuzzed_data_provider.ConsumeBool(); const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false)); const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID; if (accepted) { diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 90c1a71d9f..8241dff189 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -5,7 +5,7 @@ #include <kernel/mempool_persist.h> #include <chainparamsbase.h> -#include <mempool_args.h> +#include <node/mempool_args.h> #include <node/mempool_persist_args.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index dccc7ce795..ce23d6013d 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -141,23 +141,30 @@ BOOST_AUTO_TEST_CASE(multisig_IsStandard) for (int i = 0; i < 4; i++) key[i].MakeNewKey(true); - TxoutType whichType; + const auto is_standard{[](const CScript& spk) { + TxoutType type; + bool res{::IsStandard(spk, std::nullopt, type)}; + if (res) { + BOOST_CHECK_EQUAL(type, TxoutType::MULTISIG); + } + return res; + }}; CScript a_and_b; a_and_b << OP_2 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_2 << OP_CHECKMULTISIG; - BOOST_CHECK(::IsStandard(a_and_b, whichType)); + BOOST_CHECK(is_standard(a_and_b)); CScript a_or_b; a_or_b << OP_1 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_2 << OP_CHECKMULTISIG; - BOOST_CHECK(::IsStandard(a_or_b, whichType)); + BOOST_CHECK(is_standard(a_or_b)); CScript escrow; escrow << OP_2 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << ToByteVector(key[2].GetPubKey()) << OP_3 << OP_CHECKMULTISIG; - BOOST_CHECK(::IsStandard(escrow, whichType)); + BOOST_CHECK(is_standard(escrow)); CScript one_of_four; one_of_four << OP_1 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << ToByteVector(key[2].GetPubKey()) << ToByteVector(key[3].GetPubKey()) << OP_4 << OP_CHECKMULTISIG; - BOOST_CHECK(!::IsStandard(one_of_four, whichType)); + BOOST_CHECK(!is_standard(one_of_four)); CScript malformed[6]; malformed[0] << OP_3 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_2 << OP_CHECKMULTISIG; @@ -167,8 +174,9 @@ BOOST_AUTO_TEST_CASE(multisig_IsStandard) malformed[4] << OP_1 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()) << OP_CHECKMULTISIG; malformed[5] << OP_1 << ToByteVector(key[0].GetPubKey()) << ToByteVector(key[1].GetPubKey()); - for (int i = 0; i < 6; i++) - BOOST_CHECK(!::IsStandard(malformed[i], whichType)); + for (int i = 0; i < 6; i++) { + BOOST_CHECK(!is_standard(malformed[i])); + } } BOOST_AUTO_TEST_CASE(multisig_Sign) diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index a221e02d2f..6e810faa0a 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -18,6 +18,11 @@ #include <boost/test/unit_test.hpp> // Helpers: +bool IsStandardTx(const CTransaction& tx, std::string& reason) +{ + return IsStandardTx(tx, std::nullopt, DEFAULT_PERMIT_BAREMULTISIG, CFeeRate{DUST_RELAY_TX_FEE}, reason); +} + static std::vector<unsigned char> Serialize(const CScript& s) { @@ -49,7 +54,6 @@ BOOST_FIXTURE_TEST_SUITE(script_p2sh_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(sign) { - LOCK(cs_main); // Pay-to-script-hash looks like this: // scriptSig: <sig> <sig...> <serialized_script> // scriptPubKey: HASH160 <hash> EQUAL @@ -149,7 +153,6 @@ BOOST_AUTO_TEST_CASE(norecurse) BOOST_AUTO_TEST_CASE(set) { - LOCK(cs_main); // Test the CScript::Set* methods FillableSigningProvider keystore; CKey key[4]; @@ -263,7 +266,6 @@ BOOST_AUTO_TEST_CASE(switchover) BOOST_AUTO_TEST_CASE(AreInputsStandard) { - LOCK(cs_main); CCoinsView coinsDummy; CCoinsViewCache coins(&coinsDummy); FillableSigningProvider keystore; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 4e6c223ccc..cd6bf11327 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -40,6 +40,9 @@ typedef std::vector<unsigned char> valtype; // In script_tests.cpp UniValue read_json(const std::string& jsondata); +static CFeeRate g_dust{DUST_RELAY_TX_FEE}; +static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG}; + static std::map<std::string, unsigned int> mapFlagNames = { {std::string("P2SH"), (unsigned int)SCRIPT_VERIFY_P2SH}, {std::string("STRICTENC"), (unsigned int)SCRIPT_VERIFY_STRICTENC}, @@ -745,7 +748,6 @@ BOOST_AUTO_TEST_CASE(test_witness) BOOST_AUTO_TEST_CASE(test_IsStandard) { - LOCK(cs_main); FillableSigningProvider keystore; CCoinsView coinsDummy; CCoinsViewCache coins(&coinsDummy); @@ -765,19 +767,19 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) constexpr auto CheckIsStandard = [](const auto& t) { std::string reason; - BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK(IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason)); BOOST_CHECK(reason.empty()); }; constexpr auto CheckIsNotStandard = [](const auto& t, const std::string& reason_in) { std::string reason; - BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK(!IsStandardTx(CTransaction{t}, MAX_OP_RETURN_RELAY, g_bare_multi, g_dust, reason)); BOOST_CHECK_EQUAL(reason_in, reason); }; CheckIsStandard(t); // Check dust with default relay fee: - CAmount nDustThreshold = 182 * dustRelayFee.GetFeePerK() / 1000; + CAmount nDustThreshold = 182 * g_dust.GetFeePerK() / 1000; BOOST_CHECK_EQUAL(nDustThreshold, 546); // dust: t.vout[0].nValue = nDustThreshold - 1; @@ -805,14 +807,14 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) // Check dust with odd relay fee to verify rounding: // nDustThreshold = 182 * 3702 / 1000 - dustRelayFee = CFeeRate(3702); + g_dust = CFeeRate(3702); // dust: t.vout[0].nValue = 674 - 1; CheckIsNotStandard(t, "dust"); // not dust: t.vout[0].nValue = 674; CheckIsStandard(t); - dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); + g_dust = CFeeRate{DUST_RELAY_TX_FEE}; t.vout[0].scriptPubKey = CScript() << OP_1; CheckIsNotStandard(t, "scriptpubkey"); @@ -924,16 +926,16 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK_EQUAL(GetTransactionWeight(CTransaction(t)), 400004); CheckIsNotStandard(t, "tx-size"); - // Check bare multisig (standard if policy flag fIsBareMultisigStd is set) - fIsBareMultisigStd = true; + // Check bare multisig (standard if policy flag g_bare_multi is set) + g_bare_multi = true; t.vout[0].scriptPubKey = GetScriptForMultisig(1, {key.GetPubKey()}); // simple 1-of-1 t.vin.resize(1); t.vin[0].scriptSig = CScript() << std::vector<unsigned char>(65, 0); CheckIsStandard(t); - fIsBareMultisigStd = false; + g_bare_multi = false; CheckIsNotStandard(t, "bare-multisig"); - fIsBareMultisigStd = DEFAULT_PERMIT_BAREMULTISIG; + g_bare_multi = DEFAULT_PERMIT_BAREMULTISIG; // Check P2WPKH outputs dust threshold t.vout[0].scriptPubKey = CScript() << OP_0 << ParseHex("ffffffffffffffffffffffffffffffffffffffff"); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 67984721a3..dc6e000c65 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -14,12 +14,12 @@ #include <init.h> #include <init/common.h> #include <interfaces/chain.h> -#include <mempool_args.h> #include <net.h> #include <net_processing.h> #include <node/blockstorage.h> #include <node/chainstate.h> #include <node/context.h> +#include <node/mempool_args.h> #include <node/miner.h> #include <noui.h> #include <policy/fees.h> @@ -160,7 +160,8 @@ CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node) // chainparams.DefaultConsistencyChecks for tests .check_ratio = 1, }; - ApplyArgsManOptions(*node.args, mempool_opts); + const auto err{ApplyArgsManOptions(*node.args, ::Params(), mempool_opts)}; + Assert(!err); return mempool_opts; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 7eff6bdbe3..b151953d0d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -106,7 +106,7 @@ void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp) size_t CTxMemPoolEntry::GetTxSize() const { - return GetVirtualTransactionSize(nTxWeight, sigOpCost); + return GetVirtualTransactionSize(nTxWeight, sigOpCost, ::nBytesPerSigOp); } void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants, @@ -458,6 +458,12 @@ CTxMemPool::CTxMemPool(const Options& opts) minerPolicyEstimator{opts.estimator}, m_max_size_bytes{opts.max_size_bytes}, m_expiry{opts.expiry}, + m_incremental_relay_feerate{opts.incremental_relay_feerate}, + m_min_relay_feerate{opts.min_relay_feerate}, + m_dust_relay_feerate{opts.dust_relay_feerate}, + m_permit_bare_multisig{opts.permit_bare_multisig}, + m_max_datacarrier_bytes{opts.max_datacarrier_bytes}, + m_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, m_limits{opts.limits} { @@ -1117,12 +1123,12 @@ CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const { rollingMinimumFeeRate = rollingMinimumFeeRate / pow(2.0, (time - lastRollingFeeUpdate) / halflife); lastRollingFeeUpdate = time; - if (rollingMinimumFeeRate < (double)incrementalRelayFee.GetFeePerK() / 2) { + if (rollingMinimumFeeRate < (double)m_incremental_relay_feerate.GetFeePerK() / 2) { rollingMinimumFeeRate = 0; return CFeeRate(0); } } - return std::max(CFeeRate(llround(rollingMinimumFeeRate)), incrementalRelayFee); + return std::max(CFeeRate(llround(rollingMinimumFeeRate)), m_incremental_relay_feerate); } void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { @@ -1146,7 +1152,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends // to have 0 fee). This way, we don't allow txn to enter mempool with feerate // equal to txn which were removed with no block in between. CFeeRate removed(it->GetModFeesWithDescendants(), it->GetSizeWithDescendants()); - removed += incrementalRelayFee; + removed += m_incremental_relay_feerate; trackPackageRemoved(removed); maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed); diff --git a/src/txmempool.h b/src/txmempool.h index d7d308038c..73904cc370 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -568,6 +568,12 @@ public: const int64_t m_max_size_bytes; const std::chrono::seconds m_expiry; + const CFeeRate m_incremental_relay_feerate; + const CFeeRate m_min_relay_feerate; + const CFeeRate m_dust_relay_feerate; + const bool m_permit_bare_multisig; + const std::optional<unsigned> m_max_datacarrier_bytes; + const bool m_require_standard; const bool m_full_rbf; using Limits = kernel::MemPoolLimits; @@ -702,11 +708,11 @@ public: void CalculateDescendants(txiter it, setEntries& setDescendants) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** The minimum fee to get into the mempool, which may itself not be enough - * for larger-sized transactions. - * The incrementalRelayFee policy variable is used to bound the time it - * takes the fee rate to go back down all the way to 0. When the feerate - * would otherwise be half of this, it is set to 0 instead. - */ + * for larger-sized transactions. + * The m_incremental_relay_feerate policy variable is used to bound the time it + * takes the fee rate to go back down all the way to 0. When the feerate + * would otherwise be half of this, it is set to 0 instead. + */ CFeeRate GetMinFee() const { return GetMinFee(m_max_size_bytes); } diff --git a/src/validation.cpp b/src/validation.cpp index 17211956f5..d018d185e9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -124,7 +124,6 @@ GlobalMutex g_best_block_mutex; std::condition_variable g_best_block_cv; uint256 g_best_block; bool g_parallel_script_checks{false}; -bool fRequireStandard = true; bool fCheckBlockIndex = false; bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED; int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; @@ -647,8 +646,9 @@ private: return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } - if (package_fee < ::minRelayTxFee.GetFee(package_size)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + if (package_fee < m_pool.m_min_relay_feerate.GetFee(package_size)) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", + strprintf("%d < %d", package_fee, m_pool.m_min_relay_feerate.GetFee(package_size))); } return true; } @@ -700,8 +700,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; - if (fRequireStandard && !IsStandardTx(tx, reason)) + if (m_pool.m_require_standard && !IsStandardTx(tx, m_pool.m_max_datacarrier_bytes, m_pool.m_permit_bare_multisig, m_pool.m_dust_relay_feerate, reason)) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); + } // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. @@ -807,13 +808,14 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return false; // state filled in by CheckTxInputs } - if (fRequireStandard && !AreInputsStandard(tx, m_view)) { + if (m_pool.m_require_standard && !AreInputsStandard(tx, m_view)) { return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); } // Check for non-standard witnesses. - if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) + if (tx.HasWitness() && m_pool.m_require_standard && !IsWitnessStandard(tx, m_view)) { return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard"); + } int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -840,7 +842,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); - // No individual transactions are allowed below minRelayTxFee and mempool min fee except from + // No individual transactions are allowed below the min relay feerate and mempool min feerate except from // disconnected blocks and transactions in a package. Package transactions will be checked using // package feerate later. if (!bypass_limits && !args.m_package_feerates && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state)) return false; @@ -958,7 +960,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) ws.m_conflicting_size += it->GetTxSize(); } if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, - ::incrementalRelayFee, hash)}) { + m_pool.m_incremental_relay_feerate, hash)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } return true; diff --git a/src/validation.h b/src/validation.h index 9fef69799b..df9146852f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -100,7 +100,6 @@ extern uint256 g_best_block; * False indicates all script checking is done on the main threadMessageHandler thread. */ extern bool g_parallel_script_checks; -extern bool fRequireStandard; extern bool fCheckBlockIndex; extern bool fCheckpointsEnabled; /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 6f81fa30a1..3514d018b7 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -87,7 +87,7 @@ CFeeRate GetDiscardRate(const CWallet& wallet) CFeeRate discard_rate = wallet.chain().estimateSmartFee(highest_target, false /* conservative */); // Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate discard_rate = (discard_rate == CFeeRate(0)) ? wallet.m_discard_rate : std::min(discard_rate, wallet.m_discard_rate); - // Discard rate must be at least dustRelayFee + // Discard rate must be at least dust relay feerate discard_rate = std::max(discard_rate, wallet.chain().relayDustFee()); return discard_rate; } |