Bitcoin Core

txgraph: avoid moving primitive members

`CommitStaging()` moved primitive staging members into the main cluster set even though moving them only copies the values.

Assign the primitive members directly and keep moves for the owning containers.

coins: avoid moving `COutPoint` values

`EmplaceCoinInternalDANGER` took `COutPoint&&`, forcing callers to pass trivially copyable outpoints as rvalues even though the cache stores its own key.

Take `outpoint` by const reference and pass existing outpoints directly at all call sites.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

doc: add release note for mining option validation

ci: enforce iwyu for touched files

Except tests, fuzz and util which are left to a followup.

refactor: have mining files include what they use

mining: store block create options in NodeContext

Read configured mining options once during startup instead of parsing
options like -blockmaxweight each time a block template is generated.

Store the parsed startup options as BlockCreateOptions. Members left unset
keep representing unset options; hardcoded defaults are applied by
FlattenMiningOptions() before the options are used. IPC overrides and node
defaults can then be merged with the same option type used by
BlockAssembler.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

mining: reject invalid block create options

Check BlockCreateOptions before block template creation instead of
clamping runtime values. This makes invalid runtime block creation
options, including those passed by IPC mining clients, fail explicitly
instead of silently mining with different values than the caller
requested.

Runtime option validation now uses the same error wording as startup
option validation. Startup validation also rejects -blockmaxweight
values lower than -blockreservedweight instead of allowing them to be
clamped later.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

mining: add block create option helpers

Move block template defaulting into helper functions for
BlockCreateOptions. FlattenMiningOptions() fills hardcoded defaults and
MergeMiningOptions() overlays defaults without replacing caller-provided
values.

Use the shared option type in BlockAssembler so IPC callers and internal
callers can pass through the same options path. This commit does not
change behavior, except for dropping the "Specified " prefix from startup
option error messages.

Keep the -blockmintxfee ParseMoney check in ReadMiningArgs() instead of
CheckMiningOptions(), because CheckMiningOptions() only sees the parsed
CFeeRate value and not the original string.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

test: regression test for waitNext mining policy

Demonstrates that BlockTemplateImpl::waitNext() must respect the
mining policy supplied via -blockmintxfee, not silently fall back
to defaults.

Co-authored-by: Enoch Azariah <enirox001@gmail.com>

test: cover IPC blockmaxweight policy

Verify that -blockmaxweight is honored both when an IPC block template is first created and after waitNext() refreshes the template.

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>

move-only: add node/mining_types.h

Move mining related structs there.

This simplifies includes in later commits and makes the code easier to
understand for Mining IPC client developers.

interfaces: make Mining use const NodeContext

The next commit switches test helpers that take a const NodeContext& to
create blocks through the Mining interface, so MakeMining needs to
accept a const NodeContext too.

mining: use interface for tests, bench and fuzzers

Have most tests, benchmarks and fuzzers go through the mining interface.
This is a refactor: it does not change what blocks are created, just how
the creation calls are made. This avoids most direct test, benchmark and
fuzzer use of node::BlockAssembler::Options, making it easier to drop in a
later commit.

Two exceptions which use BlockAssembler directly:
- one check in test/miner_tests.cpp needs m_package_feerates
- fuzz/tx_pool.cpp Finish() doesn't have access to a NodeContext

Move test_block_validity from BlockAssembler::Options to
BlockCreateOptions so bench/block_assemble.cpp can continue to set it.
Just like coinbase_output_script, this is not exposed to IPC clients.

Inline options variable in places where it's only needed once.

We also drop one unused PrepareBlock declaration and one unused
implementation.

TestChain100Setup::CreateBlock no longer needs a chainstate argument,
which in turn means it can be dropped from CreateAndProcessBlock. Using
the Mining interface here also requires marking the test
KernelNotifications chainstate as loaded after LoadVerifyActivateChainstate().

miner: add block_max_weight to BlockCreateOptions

This new optional replaces nBlockMaxWeight.

Use uint64_t for the block weight options to match BlockAssembler's
nBlockWeight accounting and the IPC schema's blockReservedWeight type.

A negative -blockmaxweight value is now parsed as 0 instead of an
overflowed signed value before validation rejects it.

The new block_max_weight option is not exposed to IPC clients.

test: add createNewBlock failure helper

Move the IPC createNewBlock failure assertion into ipc_util.py so
later invalid mining option tests can share the same remote exception check.

test: misc interface_ipc_mining.py improvements

- clarify run_ipc_option_override_test description: https://github.com/bitcoin/bitcoin/pull/33965#discussion_r2784515535
- trailing comma after includes: https://github.com/bitcoin/bitcoin/pull/34452#discussion_r2775183035
- include order: https://github.com/bitcoin/bitcoin/pull/34452#discussion_r2774730966
- reuse the shared miniwallet in the low block height test: https://github.com/bitcoin/bitcoin/pull/34860#discussion_r3255163123

mining: parse block creation args in mining_args

Move the argument parsing for -blockmaxweight, -blockreservedweight,
-blockmintxfee out of init.cpp to a dedicated mining_args.cpp.

This is mostly a refactor and keeps the existing validation checks. It
does switch the weight arguments to GetArg<uint64_t>, introduced in
bitcoin/bitcoin#34582, so very large or negative weight values can be
reported differently in error messages.

refactor: Set TestNode.cli only after RPC is connected

The cli can only be called after the RPC is connected, so the cli field
should only be set after that. This is similar to the _rpc field.

This change has also other benefits:

* Manually overwriting the cli with a new rpchost is no longer needed,
so one cli-specific line can be removed from the rpc_bind.py test.
* The datadir and rpc_timeout fields are removed from the TestNodeCLI
struct. They were redundant with the -datadir and -rpcclienttimeout
command line options. Any command line option can be overwritten by
appending it with a new value, if needed.

refactor: Inline get_rpc_proxy

This is only used and needed in a single place.

test: Allow rpc_bind.py --usecli

This allows to run the test under bitcoin-cli, except for one small
part, which is skipped for now.

This also inlines rpc_url directly into create_new_rpc_connection,
because this is the only place where it is needed.

refactor: Use create_new_rpc_connection in wallet_multiwallet.py

test: Allow feature_shutdown.py --usecli

test: Allow mining_getblocktemplate_longpoll.py --usecli

Part of the diff can be reviewed with --ignore-all-space

kernel: improve BITCOINKERNEL_WARN_UNUSED_RESULT usage

Similar to [[nodiscard]], BITCOINKERNEL_WARN_UNUSED_RESULT is used
to indicate that ignoring a function's return value is almost
certainly a bug.

It is used in cases such as a resource leak (e.g. an owning handle returned
by a *_create or *_copy function), or when the returned value is itself an
error/status code. It is not used merely because discarding the result is
wasteful, e.g. on getters or predicates.

Fix the incorrect usage, and properly document the attribute.

test: suppress ECONNABORTED in wait_for_rpc_connection on Windows

Since bitcoin/bitcoin#33362, feature_bind_port_externalip.py runs in CI and
auto-detects whether 1.1.1.5 is assigned by starting nodes with -bind=1.1.1.5,
then converting a FailedToStartError containing "Unable to bind to" into a
SkipTest. On Windows CI the address is not configured, so bitcoind fails as
expected — but intermittently, while the process is shutting down, an RPC probe
raises ConnectionAbortedError (WSAECONNABORTED/WinError 10053) before
wait_for_rpc_connection() notices that the process has exited. That error was
not in the suppressed set, so it escaped and the test failed instead of being
skipped.

The intermittency is a race on TCP connection timing. If the probe connects
before the RPC port is listening, connect can fail with ECONNREFUSED. If the
TCP connection is established but is then closed abortively during shutdown,
the probe can see a connection-reset/aborted error instead. On POSIX systems
this commonly shows up as ECONNRESET ("connection reset by peer"). Winsock
also has WSAECONNABORTED for cases where an established connection is aborted
locally or otherwise terminated due to a timeout/protocol failure, in addition
to WSAECONNRESET for a reset by the remote side. This is why the
Windows-specific error may need to be suppressed separately.

Fix by treating ECONNABORTED identically to ECONNRESET, ETIMEDOUT, and
ECONNREFUSED: retry the probe rather than raising.

Fix was suggested by willcl-ark in
https://github.com/bitcoin/bitcoin/issues/35343#issuecomment-4507329622

Fixes #35343