diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 7abd4b75b2c..7a078c81cb2 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,7 @@ using interfaces::Rpc; using interfaces::WalletLoader; using kernel::ChainstateRole; using node::BlockAssembler; +using node::BlockCreateOptions; using node::BlockWaitOptions; using node::CoinbaseTx; using util::Join; @@ -867,9 +869,9 @@ public: class BlockTemplateImpl : public BlockTemplate { public: - explicit BlockTemplateImpl(BlockAssembler::Options assemble_options, + explicit BlockTemplateImpl(BlockCreateOptions create_options, std::unique_ptr block_template, - const NodeContext& node) : m_assemble_options(std::move(assemble_options)), + const NodeContext& node) : m_create_options(std::move(create_options)), m_block_template(std::move(block_template)), m_node(node) { @@ -914,8 +916,14 @@ public: std::unique_ptr waitNext(BlockWaitOptions options) override { - auto new_template = WaitAndCreateNewBlock(chainman(), notifications(), m_node.mempool.get(), m_block_template, options, m_assemble_options, m_interrupt_wait); - if (new_template) return std::make_unique(m_assemble_options, std::move(new_template), m_node); + auto new_template = WaitAndCreateNewBlock(chainman(), + notifications(), + m_node.mempool.get(), + m_block_template, + /*wait_options=*/options, + /*create_options=*/m_create_options, + /*interrupt_wait=*/m_interrupt_wait); + if (new_template) return std::make_unique(m_create_options, std::move(new_template), m_node); return nullptr; } @@ -924,7 +932,7 @@ public: InterruptWait(notifications(), m_interrupt_wait); } - const BlockAssembler::Options m_assemble_options; + const BlockCreateOptions m_create_options; const std::unique_ptr m_block_template; @@ -990,10 +998,15 @@ public: // Also wait during the final catch-up moments after IBD. if (!CooldownIfHeadersAhead(chainman(), notifications(), *maybe_tip, m_interrupt_mining)) return {}; } - - BlockAssembler::Options assemble_options{options}; - ApplyArgsManOptions(*Assert(m_node.args), assemble_options); - return std::make_unique(assemble_options, BlockAssembler{chainman().ActiveChainstate(), m_node.mempool.get(), assemble_options}.CreateNewBlock(), m_node); + const auto args_options{*Assert(ReadMiningArgs(*Assert(m_node.args)))}; + const BlockCreateOptions create_options{MergeMiningOptions(options, args_options)}; + return std::make_unique(create_options, + BlockAssembler{ + chainman().ActiveChainstate(), + m_node.mempool.get(), + create_options, + }.CreateNewBlock(), + m_node); } void interrupt() override diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 41c1d997128..e1b6a480fbd 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -76,40 +77,27 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) block.hashMerkleRoot = BlockMerkleRoot(block); } -static BlockAssembler::Options ClampOptions(BlockAssembler::Options options) +static BlockCreateOptions ClampOptions(BlockCreateOptions options) { - // Apply DEFAULT_BLOCK_RESERVED_WEIGHT and DEFAULT_BLOCK_MAX_WEIGHT when the caller left it unset. - options.block_reserved_weight = std::clamp(options.block_reserved_weight.value_or(DEFAULT_BLOCK_RESERVED_WEIGHT), MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT); + options = FlattenMiningOptions(std::move(options)); + options.block_reserved_weight = std::clamp(*options.block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT); options.coinbase_output_max_additional_sigops = std::clamp(options.coinbase_output_max_additional_sigops, 0, MAX_BLOCK_SIGOPS_COST); // Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity: // block_reserved_weight can safely exceed -blockmaxweight, but the rest of the block template will be empty. - options.block_max_weight = std::clamp(options.block_max_weight.value_or(DEFAULT_BLOCK_MAX_WEIGHT), *options.block_reserved_weight, MAX_BLOCK_WEIGHT); + options.block_max_weight = std::clamp(*options.block_max_weight, *options.block_reserved_weight, MAX_BLOCK_WEIGHT); return options; } -BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options) +BlockAssembler::BlockAssembler(Chainstate& chainstate, + const CTxMemPool* mempool, + BlockCreateOptions options) : chainparams{chainstate.m_chainman.GetParams()}, m_mempool{options.use_mempool ? mempool : nullptr}, m_chainstate{chainstate}, - m_options{ClampOptions(options)} + m_options{ClampOptions(std::move(options))} { } -void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& options) -{ - // Block resource limits - if (!options.block_max_weight) { - options.block_max_weight = args.GetArg("-blockmaxweight"); - } - if (const auto blockmintxfee{args.GetArg("-blockmintxfee")}) { - if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed}; - } - options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee); - if (!options.block_reserved_weight) { - options.block_reserved_weight = args.GetArg("-blockreservedweight"); - } -} - void BlockAssembler::resetBlock() { // Reserve space for fixed-size block header, txs count, and coinbase tx. @@ -274,7 +262,7 @@ void BlockAssembler::AddToBlock(const CTxMemPoolEntry& entry) nBlockSigOpsCost += entry.GetSigOpCost(); nFees += entry.GetFee(); - if (m_options.print_modified_fee) { + if (*m_options.print_modified_fee) { LogInfo("fee rate %s txid %s\n", CFeeRate(entry.GetModifiedFee(), entry.GetTxSize()).ToString(), entry.GetTx().GetHash().ToString()); @@ -300,7 +288,7 @@ void BlockAssembler::addChunks() while (selected_transactions.size() > 0) { // Check to see if min fee rate is still respected. - if (ByRatio{chunk_feerate_vsize} < ByRatio{m_options.blockMinFeeRate.GetFeePerVSize()}) { + if (ByRatio{chunk_feerate_vsize} < ByRatio{m_options.block_min_fee_rate->GetFeePerVSize()}) { // Everything else we might consider has a lower feerate return; } @@ -367,8 +355,8 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma KernelNotifications& kernel_notifications, CTxMemPool* mempool, const std::unique_ptr& block_template, - const BlockWaitOptions& options, - const BlockAssembler::Options& assemble_options, + const BlockWaitOptions& wait_options, + const BlockCreateOptions& create_options, bool& interrupt_wait) { // Delay calculating the current template fees, just in case a new block @@ -378,7 +366,7 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma // Alternate waiting for a new tip and checking if fees have risen. // The latter check is expensive so we only run it once per second. auto now{NodeClock::now()}; - const auto deadline = now + options.timeout; + const auto deadline = now + wait_options.timeout; const MillisecondsDouble tick{1000}; const bool allow_min_difficulty{chainman.GetParams().GetConsensus().fPowAllowMinDifficultyBlocks}; @@ -425,12 +413,12 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma * * We'll also create a new template if the tip changed during this iteration. */ - if (options.fee_threshold < MAX_MONEY || tip_changed) { + if (wait_options.fee_threshold < MAX_MONEY || tip_changed) { auto new_tmpl{BlockAssembler{ chainman.ActiveChainstate(), mempool, - assemble_options} - .CreateNewBlock()}; + create_options + }.CreateNewBlock()}; // If the tip changed, return the new template regardless of its fees. if (tip_changed) return new_tmpl; @@ -442,8 +430,8 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma // Check if fees increased enough to return the new template const CAmount new_fees = std::accumulate(new_tmpl->vTxFees.begin(), new_tmpl->vTxFees.end(), CAmount{0}); - Assume(options.fee_threshold != MAX_MONEY); - if (new_fees >= current_fees + options.fee_threshold) return new_tmpl; + Assume(wait_options.fee_threshold != MAX_MONEY); + if (new_fees >= current_fees + wait_options.fee_threshold) return new_tmpl; } now = NodeClock::now(); diff --git a/src/node/miner.h b/src/node/miner.h index b579dc68893..fcb5649f63a 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -38,8 +38,6 @@ using interfaces::BlockRef; namespace node { class KernelNotifications; -static const bool DEFAULT_PRINT_MODIFIED_FEE = false; - struct CBlockTemplate { CBlock block; @@ -79,12 +77,9 @@ private: Chainstate& m_chainstate; public: - struct Options : BlockCreateOptions { - CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE}; - bool print_modified_fee{DEFAULT_PRINT_MODIFIED_FEE}; - }; - - explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options); + explicit BlockAssembler(Chainstate& chainstate, + const CTxMemPool* mempool, + BlockCreateOptions create_options); /** Construct a new block template */ std::unique_ptr CreateNewBlock(); @@ -95,7 +90,7 @@ public: inline static std::optional m_last_block_weight{}; private: - const Options m_options; + const BlockCreateOptions m_options; // utility functions /** Clear the block's state and prepare for assembling a new block */ @@ -131,9 +126,6 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam /** Update an old GenerateCoinbaseCommitment from CreateNewBlock after the block txs have changed */ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman); -/** Apply -blockmintxfee and -blockmaxweight options from ArgsManager to BlockAssembler options. */ -void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options); - /* Compute the block's merkle root, insert or replace the coinbase transaction and the merkle root into the block */ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce); @@ -148,8 +140,8 @@ std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainma KernelNotifications& kernel_notifications, CTxMemPool* mempool, const std::unique_ptr& block_template, - const BlockWaitOptions& options, - const BlockAssembler::Options& assemble_options, + const BlockWaitOptions& wait_options, + const BlockCreateOptions& create_options, bool& interrupt_wait); /* Locks cs_main and returns the block hash and block height of the active chain if it exists; otherwise, returns nullopt.*/ diff --git a/src/node/mining_args.cpp b/src/node/mining_args.cpp index ece422a808e..ef859655cae 100644 --- a/src/node/mining_args.cpp +++ b/src/node/mining_args.cpp @@ -6,13 +6,17 @@ #include #include +#include #include #include +#include +#include #include #include #include #include +#include using common::AmountErrMsg; using util::Error; @@ -20,28 +24,60 @@ using util::Result; namespace node { -Result ReadMiningArgs(const ArgsManager& args) +Result CheckMiningOptions(const BlockCreateOptions& options, bool use_argnames) { - if (const auto arg{args.GetArg("-blockmintxfee")}) { - if (!ParseMoney(*arg)) { - return Error{AmountErrMsg("blockmintxfee", *arg)}; - } + if (options.block_max_weight && *options.block_max_weight > MAX_BLOCK_WEIGHT) { + return Error{Untranslated(strprintf("%s (%d) exceeds consensus maximum block weight (%d)", + use_argnames ? "-blockmaxweight" : "block_max_weight", + *options.block_max_weight, MAX_BLOCK_WEIGHT))}; } - - const uint64_t max_block_weight{args.GetArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT)}; - if (max_block_weight > MAX_BLOCK_WEIGHT) { - return Error{strprintf(_("Specified -blockmaxweight (%d) exceeds consensus maximum block weight (%d)"), max_block_weight, MAX_BLOCK_WEIGHT)}; + if (options.block_reserved_weight && *options.block_reserved_weight > MAX_BLOCK_WEIGHT) { + return Error{Untranslated(strprintf("%s (%d) exceeds consensus maximum block weight (%d)", + use_argnames ? "-blockreservedweight" : "block_reserved_weight", + *options.block_reserved_weight, MAX_BLOCK_WEIGHT))}; } - - const uint64_t block_reserved_weight{args.GetArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT)}; - if (block_reserved_weight > MAX_BLOCK_WEIGHT) { - return Error{strprintf(_("Specified -blockreservedweight (%d) exceeds consensus maximum block weight (%d)"), block_reserved_weight, MAX_BLOCK_WEIGHT)}; + if (options.block_reserved_weight && *options.block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) { + return Error{Untranslated(strprintf("%s (%d) is lower than minimum safety value of (%d)", + use_argnames ? "-blockreservedweight" : "block_reserved_weight", + *options.block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT))}; } - if (block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) { - return Error{strprintf(_("Specified -blockreservedweight (%d) is lower than minimum safety value of (%d)"), block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT)}; - } - return {}; } +Result ReadMiningArgs(const ArgsManager& args) +{ + BlockCreateOptions options; + if (const auto arg{args.GetArg("-blockmintxfee")}) { + std::optional block_min_tx_fee{ParseMoney(*arg)}; + if (!block_min_tx_fee) return Error{AmountErrMsg("blockmintxfee", *arg)}; + options.block_min_fee_rate = CFeeRate{*block_min_tx_fee}; + } + + if (const auto arg{args.GetBoolArg("-printpriority")}) options.print_modified_fee = *arg; + + options.block_reserved_weight = args.GetArg("-blockreservedweight"); + options.block_max_weight = args.GetArg("-blockmaxweight"); + + if (auto result{CheckMiningOptions(options, /*use_argnames=*/true)}; !result) return Error{util::ErrorString(result)}; + return options; +} + +BlockCreateOptions FlattenMiningOptions(BlockCreateOptions options) +{ + if (!options.block_min_fee_rate) options.block_min_fee_rate = CFeeRate{DEFAULT_BLOCK_MIN_TX_FEE}; + if (!options.print_modified_fee) options.print_modified_fee = DEFAULT_PRINT_MODIFIED_FEE; + if (!options.block_reserved_weight) options.block_reserved_weight = DEFAULT_BLOCK_RESERVED_WEIGHT; + if (!options.block_max_weight) options.block_max_weight = DEFAULT_BLOCK_MAX_WEIGHT; + return options; +} + +BlockCreateOptions MergeMiningOptions(BlockCreateOptions x, const BlockCreateOptions& y) +{ + if (!x.block_min_fee_rate) x.block_min_fee_rate = y.block_min_fee_rate; + if (!x.print_modified_fee) x.print_modified_fee = y.print_modified_fee; + if (!x.block_reserved_weight) x.block_reserved_weight = y.block_reserved_weight; + if (!x.block_max_weight) x.block_max_weight = y.block_max_weight; + return x; +} + } // namespace node diff --git a/src/node/mining_args.h b/src/node/mining_args.h index 01e695ee82c..e3ebf959c0e 100644 --- a/src/node/mining_args.h +++ b/src/node/mining_args.h @@ -5,13 +5,32 @@ #ifndef BITCOIN_NODE_MINING_ARGS_H #define BITCOIN_NODE_MINING_ARGS_H +#include #include class ArgsManager; namespace node { -[[nodiscard]] util::Result ReadMiningArgs(const ArgsManager& args); +static const bool DEFAULT_PRINT_MODIFIED_FEE = false; + +/** + * Read the mining options set in \p args. Returns an error if one was + * encountered. + */ +[[nodiscard]] util::Result ReadMiningArgs(const ArgsManager& args); + +/** Check option values for validity. Returns an error for invalid values. */ +[[nodiscard]] util::Result CheckMiningOptions(const BlockCreateOptions& options, bool use_argnames); + +/** Replace null optional values with their hardcoded defaults. */ +[[nodiscard]] BlockCreateOptions FlattenMiningOptions(BlockCreateOptions options); + +/** + * Merge two BlockCreateOptions structs, replacing null values in \p x with + * non-null values from \p y. + */ +[[nodiscard]] BlockCreateOptions MergeMiningOptions(BlockCreateOptions x, const BlockCreateOptions& y); } // namespace node diff --git a/src/node/mining_types.h b/src/node/mining_types.h index 30206c2e87c..6e3ee0f4afb 100644 --- a/src/node/mining_types.h +++ b/src/node/mining_types.h @@ -12,7 +12,7 @@ #define BITCOIN_NODE_MINING_TYPES_H #include -#include +#include #include #include #include