mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-12 21:52:53 +02:00
a4157fc24a
1e5d3b4f0ddoc: add release note for mining option validation (Sjors Provoost)0317f52022ci: enforce iwyu for touched files (Sjors Provoost)8c58f63578refactor: have mining files include what they use (Sjors Provoost)3bb6498fb0mining: store block create options in NodeContext (Sjors Provoost)4637cd157dmining: reject invalid block create options (Sjors Provoost)8daac1d6ebmining: add block create option helpers (Sjors Provoost)128da7c3ffminer: add block_max_weight to BlockCreateOptions (Sjors Provoost)fa81e51eaemining: parse block creation args in mining_args (Sjors Provoost)020166080cmining: use interface for tests, bench and fuzzers (Sjors Provoost)44082bea47interfaces: make Mining use const NodeContext (Sjors Provoost)d4368e059cmove-only: add node/mining_types.h (Sjors Provoost)6aeb1fbea2test: cover IPC blockmaxweight policy (Sjors Provoost)63b23ea1e9test: regression test for waitNext mining policy (Sjors Provoost)24750f8b31test: add createNewBlock failure helper (Sjors Provoost)63ee9cd15btest: misc interface_ipc_mining.py improvements (Sjors Provoost) Pull request description: Although this PR is primarily a refactor, _there are behavior changes_ documented in the release note: - the IPC mining interface now rejects out-of-range block template options instead of silently clamping them; - startup now rejects `-blockmaxweight` values lower than `-blockreservedweight`, instead of allowing them to be clamped later. The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing. They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of `BlockCreateOptions`. `BlockCreateOptions` is used by interface clients. As before, IPC clients have access to a safe / sane subset, whereas RPC and test code can use all fields. The same type is also used to store mining defaults parsed once during node startup in `NodeContext`. The maximum block weight setting (`block_max_weight`) is optional. When read from startup options it matches `-blockmaxweight`; when provided by callers it is a runtime override. `Merge()` fills unset fields from startup defaults while preserving caller-provided values. This all happens in commits `mining: add block create option helpers` and `mining: store block create options in NodeContext`, and requires some preparation to keep things easy to review. We get rid of `BlockAssembler::Options` but this is used in many tests. Since large churn is inevitable, we might as well switch all tests, bench and fuzzers over to the Mining interface. The `mining: use interface for tests, bench and fuzzers` commit does that, dramatically reducing direct use of `BlockAssembler`. Two exceptions are documented in the commit message. Because `test_block_validity` wasn't available via the interface and the block_assemble benchmark needs it, it's moved from `BlockAssembler::Options` to `BlockCreateOptions` (still not exposed via IPC). We need access to mining related structs from both the miner and node initialization code. To avoid having to pull in all of `BlockAssembler` for the latter, the `move-only: add node/mining_types.h` commit introduces `node/mining_types.h` and moves `BlockCreateOptions`, `BlockWaitOptions` and `BlockCheckOptions` there from `src/node/types.h`. I considered also moving `DEFAULT_BLOCK_MAX_WEIGHT`, `DEFAULT_BLOCK_RESERVED_WEIGHT`, `MINIMUM_BLOCK_RESERVED_WEIGHT` and `DEFAULT_BLOCK_MIN_TX_FEE` there from `policy.h`, since they are distinct from relay policy and not needed by the kernel. But this seems more appropriate for a follow-up and requires additional discussion. --- I kept variable renaming and other formatting changes to a minimum to ease review with `--color-moved=dimmed-zebra`. ## Commit summary Tests and test cleanup: - `test: misc interface_ipc_mining.py improvements` - `test: add assert_create_fails helper` - `test: regression test for waitNext mining policy` - `test: cover IPC blockmaxweight policy` Refactoring test/bench/fuzz callers: - `interfaces: make Mining use const NodeContext` - `mining: use interface for tests, bench and fuzzers` Moving mining interface types: - `move-only: add node/mining_types.h` Separating startup defaults from runtime options: - `mining: parse block creation args in mining_args`: adds `node/mining_args.{h,cpp}` and moves mining option parsing out of `init.cpp`, without storing the parsed values yet. - `miner: add block_max_weight to BlockCreateOptions`: moves the runtime maximum block weight setting into `BlockCreateOptions` as an optional value, so it can later be defaulted from startup args when unset. - `mining: add block create option helpers`: centralizes block template option defaulting and merging, removes `BlockAssembler::Options`, and preserves behavior except for dropping the `Specified ` prefix from startup option error messages. - `mining: reject invalid block create options`: checks typed `BlockCreateOptions` before block template creation, so invalid runtime options are rejected instead of silently clamped. Startup validation also rejects `-blockmaxweight` values lower than `-blockreservedweight`. - `mining: store block create options in NodeContext`: stores the startup mining options in `NodeContext` as `BlockCreateOptions`, so startup defaults and runtime overrides can be merged with the same option type. Include hygiene, CI and release note: - `refactor: have mining files include what they use` - `ci: enforce iwyu for touched files` - `doc: add release note for mining option validation` ACKs for top commit: w0xlt: reACK1e5d3b4f0dsedited: ACK1e5d3b4f0dryanofsky: Code review ACK1e5d3b4f0d. Looks good, thanks for the updates! Tree-SHA512: 28c715023cb78f02775caa787b243c994bd0f8ce4559afc8db9301e93400ebbc74963626a4afe65ae15bcc16b9192d051a745839f4c804848d50746ea5a224b4