From fa468bdfb62dec286cb977db78d3e47b64dafeba Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 20 Jul 2022 15:48:52 +0200 Subject: [PATCH 01/13] Return optional error from ApplyArgsManOptions Also pass in a (for now unused) reference to the params. Both changes are needed for the next commit. --- ci/test/06_script_b.sh | 1 + src/init.cpp | 4 +++- src/mempool_args.cpp | 10 +++++++++- src/mempool_args.h | 7 ++++++- src/test/util/setup_common.cpp | 3 ++- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 1220b15d5a2..978830b7e2b 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -44,6 +44,7 @@ if [ "${RUN_TIDY}" = "true" ]; then " src/dbwrapper.cpp"\ " src/init"\ " src/kernel"\ + " src/mempool_args.cpp"\ " src/node/chainstate.cpp"\ " src/policy/feerate.cpp"\ " src/policy/packages.cpp"\ diff --git a/src/init.cpp b/src/init.cpp index a94bbe6460e..9a3c70abbd0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1418,7 +1418,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(mempool_opts.check_ratio, 0, 1'000'000); int64_t descendant_limit_bytes = mempool_opts.limits.descendant_size_vbytes * 40; diff --git a/src/mempool_args.cpp b/src/mempool_args.cpp index 77caa127e96..da0fb622623 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -7,7 +7,13 @@ #include #include +#include +#include #include +#include + +#include +#include using kernel::MemPoolLimits; using kernel::MemPoolOptions; @@ -25,7 +31,7 @@ void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolLimits& mempool_limi } } -void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolOptions& mempool_opts) +std::optional ApplyArgsManOptions(const ArgsManager& argsman, const CChainParams& chainparams, MemPoolOptions& mempool_opts) { mempool_opts.check_ratio = argsman.GetIntArg("-checkmempool", mempool_opts.check_ratio); @@ -36,4 +42,6 @@ void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolOptions& mempool_opt 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/mempool_args.h index 9a4abe6618f..72d202cca27 100644 --- a/src/mempool_args.h +++ b/src/mempool_args.h @@ -5,18 +5,23 @@ #ifndef BITCOIN_MEMPOOL_ARGS_H #define BITCOIN_MEMPOOL_ARGS_H +#include + 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 ApplyArgsManOptions(const ArgsManager& argsman, const CChainParams& chainparams, kernel::MemPoolOptions& mempool_opts); #endif // BITCOIN_MEMPOOL_ARGS_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 67984721a37..f5076c043bb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -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; } From fa148602e67fe035b1b21eff6c0b656919ac2d45 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 20 Jul 2022 15:59:03 +0200 Subject: [PATCH 02/13] Remove ::fRequireStandard global --- src/init.cpp | 4 ---- src/kernel/mempool_options.h | 1 + src/mempool_args.cpp | 5 +++++ src/test/fuzz/tx_pool.cpp | 9 ++++----- src/txmempool.cpp | 1 + src/txmempool.h | 1 + src/validation.cpp | 9 +++++---- src/validation.h | 1 - test/functional/p2p_segwit.py | 6 +++--- 9 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 9a3c70abbd0..1539f71aa7c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1004,10 +1004,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb } } - 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; diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 07953b443bc..861860f345c 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -33,6 +33,7 @@ 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}}; + 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 index da0fb622623..942c8758479 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -39,6 +39,11 @@ std::optional ApplyArgsManOptions(const ArgsManager& argsman, con if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours}; + 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); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index cfb112879af..b1db1a425fd 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -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(&tx_pool_); chainstate.SetMempool(&tx_pool); @@ -237,7 +238,6 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) auto txr = std::make_shared(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(&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/txmempool.cpp b/src/txmempool.cpp index 7eff6bdbe32..f520f4705c9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -458,6 +458,7 @@ CTxMemPool::CTxMemPool(const Options& opts) minerPolicyEstimator{opts.estimator}, m_max_size_bytes{opts.max_size_bytes}, m_expiry{opts.expiry}, + m_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, m_limits{opts.limits} { diff --git a/src/txmempool.h b/src/txmempool.h index d7d308038c2..6d04a4a6eba 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -568,6 +568,7 @@ public: const int64_t m_max_size_bytes; const std::chrono::seconds m_expiry; + const bool m_require_standard; const bool m_full_rbf; using Limits = kernel::MemPoolLimits; diff --git a/src/validation.cpp b/src/validation.cpp index 17211956f57..271ca765c6d 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; @@ -700,8 +699,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, 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 +807,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); diff --git a/src/validation.h b/src/validation.h index 9fef69799be..df9146852f8 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/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index db6954ccf65..311b0b67dbd 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -245,7 +245,7 @@ class SegWitTest(BitcoinTestFramework): self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) # self.old_node sets only NODE_NETWORK self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK) - # self.std_node is for testing node1 (fRequireStandard=true) + # self.std_node is for testing node1 (requires standard txs) self.std_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) # self.std_wtx_node is for testing node1 with wtxid relay self.std_wtx_node = self.nodes[1].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=P2P_SERVICES) @@ -1382,7 +1382,7 @@ class SegWitTest(BitcoinTestFramework): tx3.vout.append(CTxOut(total_value - 1000, script_pubkey)) tx3.rehash() - # First we test this transaction against fRequireStandard=true node + # First we test this transaction against std_node # making sure the txid is added to the reject filter self.std_node.announce_tx_and_wait_for_getdata(tx3) test_transaction_acceptance(self.nodes[1], self.std_node, tx3, with_witness=True, accepted=False, reason="bad-txns-nonstandard-inputs") @@ -1390,7 +1390,7 @@ class SegWitTest(BitcoinTestFramework): self.std_node.announce_tx_and_wait_for_getdata(tx3, success=False) # Spending a higher version witness output is not allowed by policy, - # even with fRequireStandard=false. + # even with the node that accepts non-standard txs. test_transaction_acceptance(self.nodes[0], self.test_node, tx3, with_witness=True, accepted=False, reason="reserved for soft-fork upgrades") # Building a block with the transaction must be valid, however. From fa9cba7afb73c01bd2c8fefd662dfc80dd98c5e8 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 11:40:22 +0200 Subject: [PATCH 03/13] Remove ::incrementalRelayFee and ::minRelayTxFee globals --- doc/policy/README.md | 2 +- doc/policy/mempool-replacements.md | 2 +- doc/policy/packages.md | 2 +- src/init.cpp | 23 ----------------------- src/kernel/mempool_options.h | 6 ++++++ src/mempool_args.cpp | 28 ++++++++++++++++++++++++++++ src/net_processing.cpp | 4 ++-- src/node/interfaces.cpp | 12 ++++++++++-- src/policy/policy.h | 4 ++-- src/policy/settings.cpp | 2 -- src/policy/settings.h | 4 ---- src/rpc/fees.cpp | 3 +-- src/rpc/mempool.cpp | 6 +++--- src/rpc/net.cpp | 7 +++++-- src/txmempool.cpp | 8 +++++--- src/txmempool.h | 12 +++++++----- src/validation.cpp | 9 +++++---- 17 files changed, 77 insertions(+), 57 deletions(-) diff --git a/doc/policy/README.md b/doc/policy/README.md index 6e8686365dc..27536407e7d 100644 --- a/doc/policy/README.md +++ b/doc/policy/README.md @@ -3,7 +3,7 @@ **Policy** (Mempool or Transaction Relay Policy) is the node's set of validation rules, in addition to consensus, enforced for unconfirmed transactions before submitting them to the mempool. These rules are local to the node and configurable (e.g. `-minrelaytxfee`, `-limitancestorsize`, -`-incrementalRelayFee`). Policy may include restrictions on the transaction itself, the transaction +`-incrementalrelayfee`). Policy may include restrictions on the transaction itself, the transaction in relation to the current chain tip, and the transaction in relation to the node's mempool contents. Policy is *not* applied to transactions in blocks. diff --git a/doc/policy/mempool-replacements.md b/doc/policy/mempool-replacements.md index 430a96f228c..b3c4239b731 100644 --- a/doc/policy/mempool-replacements.md +++ b/doc/policy/mempool-replacements.md @@ -71,7 +71,7 @@ This set of rules is similar but distinct from BIP125. Bitcoin Core implementation. * The incremental relay feerate used to calculate the required additional fees is distinct from - `minRelayTxFee` and configurable using `-incrementalrelayfee` + `-minrelaytxfee` and configurable using `-incrementalrelayfee` ([PR #9380](https://github.com/bitcoin/bitcoin/pull/9380)). * RBF enabled by default in the wallet GUI as of **v0.18.1** ([PR diff --git a/doc/policy/packages.md b/doc/policy/packages.md index f2a3d6517ef..03c26da86be 100644 --- a/doc/policy/packages.md +++ b/doc/policy/packages.md @@ -81,7 +81,7 @@ If any transactions in the package are already in the mempool, they are not subm ("deduplicated") and are thus excluded from this calculation. To meet the two feerate requirements of a mempool, i.e., the pre-configured minimum relay feerate -(`minRelayTxFee`) and the dynamic mempool minimum feerate, the total package feerate is used instead +(`-minrelaytxfee`) and the dynamic mempool minimum feerate, the total package feerate is used instead of the individual feerate. The individual transactions are allowed to be below the feerate requirements if the package meets the feerate requirements. For example, the parent(s) in the package can pay no fees but be paid for by the child. diff --git a/src/init.cpp b/src/init.cpp index 1539f71aa7c..b358a59dc66 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -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 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 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")) { diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 861860f345c..3e9be06a779 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -6,6 +6,9 @@ #include +#include +#include + #include #include @@ -33,6 +36,9 @@ 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}; 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 index 942c8758479..f67eb993c87 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -8,7 +8,12 @@ #include #include +#include +#include +#include #include +#include +#include #include #include @@ -39,6 +44,29 @@ std::optional ApplyArgsManOptions(const ArgsManager& argsman, con 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 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 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()); + } + 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()); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0e10fa5f9d5..2e113905416 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 699dca0a731..597b473d4fe 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -676,8 +676,16 @@ public: if (!m_node.mempool) return {}; return m_node.mempool->GetMinFee(); } - CFeeRate relayMinFee() override { return ::minRelayTxFee; } - CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; } + 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 { return ::dustRelayFee; } bool havePruned() override { diff --git a/src/policy/policy.h b/src/policy/policy.h index cd98a601a31..15a66efa0de 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 */ diff --git a/src/policy/settings.cpp b/src/policy/settings.cpp index 0b67d274ce8..a4177f71845 100644 --- a/src/policy/settings.cpp +++ b/src/policy/settings.cpp @@ -9,7 +9,5 @@ #include 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 2311d01fe8b..eb26a825a8c 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -14,11 +14,7 @@ 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; diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index 41f386d4430..aa047bdea87 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -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 0ae10b6c39e..02b51ce0a04 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 059be61bb43..0ee905a77a9 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/txmempool.cpp b/src/txmempool.cpp index f520f4705c9..5a47700bc82 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -458,6 +458,8 @@ 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_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, m_limits{opts.limits} @@ -1118,12 +1120,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) { @@ -1147,7 +1149,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* 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 6d04a4a6eba..af5e95a3ecf 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -568,6 +568,8 @@ 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 bool m_require_standard; const bool m_full_rbf; @@ -703,11 +705,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 271ca765c6d..cd14dec7c9e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -646,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; } @@ -841,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; @@ -959,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; From fa7a9114e59b81b50584311a4ab2b3e9a8d956bd Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 12:18:32 +0200 Subject: [PATCH 04/13] test: Remove unused cs_main --- src/test/script_p2sh_tests.cpp | 3 --- src/test/transaction_tests.cpp | 1 - 2 files changed, 4 deletions(-) diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index a221e02d2f9..53a7c18d4a2 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -49,7 +49,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: // scriptPubKey: HASH160 EQUAL @@ -149,7 +148,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 +261,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 4e6c223ccc4..d26286923cd 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -745,7 +745,6 @@ BOOST_AUTO_TEST_CASE(test_witness) BOOST_AUTO_TEST_CASE(test_IsStandard) { - LOCK(cs_main); FillableSigningProvider keystore; CCoinsView coinsDummy; CCoinsViewCache coins(&coinsDummy); From fa8a7f01fe1b6db98097021276ed5d929faadbec Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 13:12:58 +0200 Subject: [PATCH 05/13] Remove ::IsStandardTx(tx, reason) alias Apart from tests, it is only used in one place, so there is no need for an alias. --- src/policy/settings.h | 6 ------ src/test/fuzz/transaction.cpp | 1 - src/test/script_p2sh_tests.cpp | 5 +++++ src/test/transaction_tests.cpp | 21 ++++++++++++--------- src/validation.cpp | 2 +- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/policy/settings.h b/src/policy/settings.h index eb26a825a8c..0f82969f7e3 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -10,7 +10,6 @@ #include #include -#include class CTransaction; @@ -18,11 +17,6 @@ extern CFeeRate dustRelayFee; 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); diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 273aa0dc5c8..01c30e9b2a2 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -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/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index 53a7c18d4a2..ece303877f4 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -18,6 +18,11 @@ #include // Helpers: +bool IsStandardTx(const CTransaction& tx, std::string& reason) +{ + return IsStandardTx(tx, DEFAULT_PERMIT_BAREMULTISIG, CFeeRate{DUST_RELAY_TX_FEE}, reason); +} + static std::vector Serialize(const CScript& s) { diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index d26286923cd..9055b8f559a 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -40,6 +40,9 @@ typedef std::vector 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 mapFlagNames = { {std::string("P2SH"), (unsigned int)SCRIPT_VERIFY_P2SH}, {std::string("STRICTENC"), (unsigned int)SCRIPT_VERIFY_STRICTENC}, @@ -764,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}, 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}, 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; @@ -804,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"); @@ -923,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(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/validation.cpp b/src/validation.cpp index cd14dec7c9e..0840e3fc426 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -700,7 +700,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; - if (m_pool.m_require_standard && !IsStandardTx(tx, reason)) { + if (m_pool.m_require_standard && !IsStandardTx(tx, ::fIsBareMultisigStd, ::dustRelayFee, reason)) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); } From fadc14e4f514e7167723285e0ac3d4a7149bbee6 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 13:47:36 +0200 Subject: [PATCH 06/13] Remove ::dustRelayFee --- src/init.cpp | 10 ---------- src/kernel/mempool_options.h | 1 + src/mempool_args.cpp | 10 ++++++++++ src/node/interfaces.cpp | 12 ++++++++++-- src/policy/settings.cpp | 2 -- src/policy/settings.h | 2 -- src/txmempool.cpp | 1 + src/txmempool.h | 1 + src/validation.cpp | 2 +- src/wallet/fees.cpp | 2 +- 10 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index b358a59dc66..a0e2b7fcfd5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -971,16 +971,6 @@ 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 parsed = ParseMoney(args.GetArg("-dustrelayfee", ""))) { - dustRelayFee = CFeeRate{parsed.value()}; - } else { - return InitError(AmountErrMsg("dustrelayfee", args.GetArg("-dustrelayfee", ""))); - } - } - nBytesPerSigOp = args.GetIntArg("-bytespersigop", nBytesPerSigOp); if (!g_wallet_init_interface.ParameterInteraction()) return false; diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 3e9be06a779..384532ae398 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -39,6 +39,7 @@ struct MemPoolOptions { 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}; 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 index f67eb993c87..745d7b03169 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -67,6 +67,16 @@ std::optional ApplyArgsManOptions(const ArgsManager& argsman, con 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 parsed = ParseMoney(argsman.GetArg("-dustrelayfee", ""))) { + mempool_opts.dust_relay_feerate = CFeeRate{parsed.value()}; + } else { + return AmountErrMsg("dustrelayfee", argsman.GetArg("-dustrelayfee", "")); + } + } + 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()); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 597b473d4fe..2c845d01274 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; @@ -686,7 +690,11 @@ public: if (!m_node.mempool) return CFeeRate{DEFAULT_INCREMENTAL_RELAY_FEE}; return m_node.mempool->m_incremental_relay_feerate; } - CFeeRate relayDustFee() override { return ::dustRelayFee; } + 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/policy/settings.cpp b/src/policy/settings.cpp index a4177f71845..6b15d31d686 100644 --- a/src/policy/settings.cpp +++ b/src/policy/settings.cpp @@ -5,9 +5,7 @@ #include -#include #include bool fIsBareMultisigStd = DEFAULT_PERMIT_BAREMULTISIG; -CFeeRate dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); unsigned int nBytesPerSigOp = DEFAULT_BYTES_PER_SIGOP; diff --git a/src/policy/settings.h b/src/policy/settings.h index 0f82969f7e3..df0bfff3945 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -6,14 +6,12 @@ #ifndef BITCOIN_POLICY_SETTINGS_H #define BITCOIN_POLICY_SETTINGS_H -#include #include #include class CTransaction; -extern CFeeRate dustRelayFee; extern unsigned int nBytesPerSigOp; extern bool fIsBareMultisigStd; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5a47700bc82..4745b99620b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -460,6 +460,7 @@ CTxMemPool::CTxMemPool(const Options& opts) 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_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, m_limits{opts.limits} diff --git a/src/txmempool.h b/src/txmempool.h index af5e95a3ecf..84e5e9ed482 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -570,6 +570,7 @@ public: 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_require_standard; const bool m_full_rbf; diff --git a/src/validation.cpp b/src/validation.cpp index 0840e3fc426..22913b763ba 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -700,7 +700,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; - if (m_pool.m_require_standard && !IsStandardTx(tx, ::fIsBareMultisigStd, ::dustRelayFee, reason)) { + if (m_pool.m_require_standard && !IsStandardTx(tx, ::fIsBareMultisigStd, m_pool.m_dust_relay_feerate, reason)) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); } diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 6f81fa30a1d..3514d018b7d 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; } From fa2f6c1a611dffe5a3f63fe1b453f1dd420371b1 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 16:20:16 +0200 Subject: [PATCH 07/13] Remove ::fIsBareMultisigStd global --- src/init.cpp | 1 - src/kernel/mempool_options.h | 1 + src/mempool_args.cpp | 3 +++ src/policy/settings.cpp | 1 - src/policy/settings.h | 1 - src/txmempool.cpp | 1 + src/txmempool.h | 1 + src/validation.cpp | 2 +- 8 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a0e2b7fcfd5..49c7de78163 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -975,7 +975,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb 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); diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 384532ae398..197a018ba3a 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -40,6 +40,7 @@ struct MemPoolOptions { /** 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}; + 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 index 745d7b03169..463ad778698 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -77,6 +78,8 @@ std::optional ApplyArgsManOptions(const ArgsManager& argsman, con } } + mempool_opts.permit_bare_multisig = argsman.GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG); + 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()); diff --git a/src/policy/settings.cpp b/src/policy/settings.cpp index 6b15d31d686..39e00f1111a 100644 --- a/src/policy/settings.cpp +++ b/src/policy/settings.cpp @@ -7,5 +7,4 @@ #include -bool fIsBareMultisigStd = DEFAULT_PERMIT_BAREMULTISIG; unsigned int nBytesPerSigOp = DEFAULT_BYTES_PER_SIGOP; diff --git a/src/policy/settings.h b/src/policy/settings.h index df0bfff3945..22af063b4c3 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -13,7 +13,6 @@ class CTransaction; extern unsigned int nBytesPerSigOp; -extern bool fIsBareMultisigStd; static inline int64_t GetVirtualTransactionSize(int64_t weight, int64_t sigop_cost) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4745b99620b..4af4ca08514 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -461,6 +461,7 @@ CTxMemPool::CTxMemPool(const Options& opts) 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_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, m_limits{opts.limits} diff --git a/src/txmempool.h b/src/txmempool.h index 84e5e9ed482..32c404d0dd0 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -571,6 +571,7 @@ public: 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 bool m_require_standard; const bool m_full_rbf; diff --git a/src/validation.cpp b/src/validation.cpp index 22913b763ba..c22b96d56fb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -700,7 +700,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; - if (m_pool.m_require_standard && !IsStandardTx(tx, ::fIsBareMultisigStd, m_pool.m_dust_relay_feerate, reason)) { + if (m_pool.m_require_standard && !IsStandardTx(tx, m_pool.m_permit_bare_multisig, m_pool.m_dust_relay_feerate, reason)) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); } From fa477d32eefcc3dd2f06b452066290d9936d8c5d Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 16:52:14 +0200 Subject: [PATCH 08/13] Remove ::GetVirtualTransactionSize() alias Each alias is only used in one place. --- src/node/psbt.cpp | 2 +- src/policy/settings.h | 16 ---------------- src/test/fuzz/integer.cpp | 3 --- src/txmempool.cpp | 2 +- 4 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 5a932f435d4..57162cd679b 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/settings.h b/src/policy/settings.h index 22af063b4c3..f0d6f779aea 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -6,22 +6,6 @@ #ifndef BITCOIN_POLICY_SETTINGS_H #define BITCOIN_POLICY_SETTINGS_H -#include - -#include - -class CTransaction; - extern unsigned int nBytesPerSigOp; -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/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 72574612a20..c52fca5fe85 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(::nBytesPerSigOp)) && !AdditionOverflow(i64 * ::nBytesPerSigOp, static_cast(4))) { - (void)GetVirtualTransactionSize(i64, i64); - } if (!MultiplicationOverflow(i64, static_cast(u32)) && !AdditionOverflow(i64, static_cast(4)) && !AdditionOverflow(i64 * u32, static_cast(4))) { (void)GetVirtualTransactionSize(i64, i64, u32); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4af4ca08514..37c6c62ea41 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, From fa2a6b8516b24d7e9ca11926a49cf2b07f661e81 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 17:55:49 +0200 Subject: [PATCH 09/13] Combine datacarrier globals into one --- src/init.cpp | 7 +++++-- src/policy/policy.cpp | 7 ++++--- src/script/standard.cpp | 3 +-- src/script/standard.h | 11 ++++++----- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 49c7de78163..04873d1c29f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -975,8 +975,11 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb if (!g_wallet_init_interface.ParameterInteraction()) return false; - fAcceptDatacarrier = args.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER); - nMaxDatacarrierBytes = args.GetIntArg("-datacarriersize", nMaxDatacarrierBytes); + if (args.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) { + g_max_datacarrier_bytes = args.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY); + } else { + g_max_datacarrier_bytes = std::nullopt; + } // Option to startup with mocktime set (used for regression testing): SetMockTime(args.GetIntArg("-mocktime", 0)); // SetMockTime(0) is a no-op diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index f6452266b7f..0a79c0a1d8d 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -82,9 +82,10 @@ 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 (!g_max_datacarrier_bytes || scriptPubKey.size() > *g_max_datacarrier_bytes) { + return false; + } } return true; diff --git a/src/script/standard.cpp b/src/script/standard.cpp index b3f6a1b6698..47e9e89c941 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -16,8 +16,7 @@ typedef std::vector valtype; -bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; -unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; +std::optional g_max_datacarrier_bytes{DEFAULT_ACCEPT_DATACARRIER ? std::optional{MAX_OP_RETURN_RELAY} : std::nullopt}; CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in)) {} CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast(in)) {} diff --git a/src/script/standard.h b/src/script/standard.h index 448fdff0109..c1c4cf4a39f 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -33,7 +34,7 @@ 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; @@ -41,11 +42,11 @@ 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. + * + * Maximum size of TxoutType::NULL_DATA scripts that this node considers standard. + * If nullopt, any size is nonstandard. */ -extern bool fAcceptDatacarrier; - -/** Maximum size of TxoutType::NULL_DATA scripts that this node considers standard. */ -extern unsigned nMaxDatacarrierBytes; +extern std::optional g_max_datacarrier_bytes; /** * Mandatory script verification flags that all new blocks must comply with for From fad0b4fab849eb5f1f0aa54ebc290f85a473ec91 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 18:36:20 +0200 Subject: [PATCH 10/13] Pass datacarrier setting into IsStandard --- src/policy/policy.cpp | 6 +++--- src/policy/policy.h | 2 +- src/test/fuzz/key.cpp | 4 ++-- src/test/fuzz/script.cpp | 2 +- src/test/multisig_tests.cpp | 22 +++++++++++++++------- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 0a79c0a1d8d..fc890bff381 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& max_datacarrier_bytes, TxoutType& whichType) { std::vector > vSolutions; whichType = Solver(scriptPubKey, vSolutions); @@ -83,7 +83,7 @@ bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType) if (m < 1 || m > n) return false; } else if (whichType == TxoutType::NULL_DATA) { - if (!g_max_datacarrier_bytes || scriptPubKey.size() > *g_max_datacarrier_bytes) { + if (!max_datacarrier_bytes || scriptPubKey.size() > *max_datacarrier_bytes) { return false; } } @@ -131,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, g_max_datacarrier_bytes, whichType)) { reason = "scriptpubkey"; return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 15a66efa0de..e3734003ce1 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -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& max_datacarrier_bytes, TxoutType& whichType); // Changing the default transaction version requires a two step process: first diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index bfea9778f4d..6d2d2e2bc58 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/script.cpp b/src/test/fuzz/script.cpp index fdcd0da37de..69a4b782aaa 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/multisig_tests.cpp b/src/test/multisig_tests.cpp index dccc7ce7956..ce23d6013d9 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) From 66664384a6fec39ecb4d8d06db66a4f193a06e33 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Thu, 21 Jul 2022 18:09:02 +0200 Subject: [PATCH 11/13] Remove ::g_max_datacarrier_bytes global --- src/init.cpp | 6 ------ src/kernel/mempool_options.h | 10 ++++++++++ src/mempool_args.cpp | 6 ++++++ src/policy/policy.cpp | 4 ++-- src/policy/policy.h | 2 +- src/script/standard.cpp | 2 -- src/script/standard.h | 10 ---------- src/test/fuzz/transaction.cpp | 4 ++-- src/test/script_p2sh_tests.cpp | 2 +- src/test/transaction_tests.cpp | 4 ++-- src/txmempool.cpp | 1 + src/txmempool.h | 1 + src/validation.cpp | 2 +- 13 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 04873d1c29f..056aadb675d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -975,12 +975,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb if (!g_wallet_init_interface.ParameterInteraction()) return false; - if (args.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) { - g_max_datacarrier_bytes = args.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY); - } else { - g_max_datacarrier_bytes = std::nullopt; - } - // Option to startup with mocktime set (used for regression testing): SetMockTime(args.GetIntArg("-mocktime", 0)); // SetMockTime(0) is a no-op diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 197a018ba3a..dad6f14c391 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -8,9 +8,11 @@ #include #include +#include