62fc42d475interfaces: refactor: move `waitTipChanged` implementation to miner (ismaelsadeeq)c39ca9d4f7interfaces: move getTip implementation to miner (Sjors Provoost)720f201e65interfaces: refactor: move `waitNext` implementation to miner (ismaelsadeeq)e6c2f4ce7ainterfaces: refactor: move `submitSolution` implementation to miner (ismaelsadeeq)02d4bc776binterfaces: remove redundant coinbase fee check in `waitNext` (ismaelsadeeq) Pull request description: #### Motivation In [Internal interface guidelines](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-guidelines) It's stated that > Interface method definitions should wrap existing functionality instead of implementing new functionality. Any substantial new node or wallet functionality should be implemented in [src/node/](https://github.com/bitcoin/bitcoin/blob/master/src/node) or [src/wallet/](https://github.com/bitcoin/bitcoin/blob/master/src/wallet) and just exposed in [src/interfaces/](https://github.com/bitcoin/bitcoin/blob/master/src/interfaces) instead of being implemented there, so it can be more modular and accessible to unit tests. However the some methods in the newly added `BlockTemplateImpl` and `MinerImpl` classes partially enforces this guideline, as the implementations of the `submitSolution`, `waitNext`, and `waitTipChanged` methods reside within the class itself. #### What the PR Does This PR introduces a simple refactor by moving certain method implementations from `BlockTemplateImpl` into the miner module. It introduces three new functions: 1. Remove rundundant coinbase fee check in `waitNext` 2. **`AddMerkleRootAndCoinbase`**: Computes the block's Merkle root, inserts the coinbase transaction, and sets the Merkle root in the block. This function is called by `submitSolution` before the block is submitted for processing. 3. **`WaitAndCreateNewBlock`**: Returns a new block template either when transaction fees reach a certain threshold or when a new tip is detected. If a timeout is reached, it returns `nullptr`. The `waitNext` method in `BlockTemplateImpl` now simply wraps this function. 4. Move `GetTip` implementation to miner. 5. **`WaitTipChanged`**: Returns the tip when the chain it changes, or `nullopt` if a timeout or interrupt occurs. The `waitTipChanged` method in `MinerImpl` now calls `GetTip` after invoking `ChainTipChanged`, and returns the tip. #### Behavior Change - We now only `Assert` for a valid chainman and notifications pointer once. ACKs for top commit: achow101: ACK62fc42d475Sjors: ACK62fc42d475ryanofsky: Code review ACK62fc42d475. Lots of suggest suggest changes made since last review, altering function names and signatures and also adding new commit to drop negative fee handling. I like the idea of making the wait function return a BlockRef, that is clearer than what I suggested. Left some comments below but they are not important and this looks good as-is Tree-SHA512: 502632f94ced81f576b2c43cf015f1527e2c259e6ca253f670f5a6889171e2246372b4e709575701afa3f01d488d6633557fef54f48fe83bbaf1836ac5326c4f
src/node/
The src/node/ directory contains code that needs to access node state
(state in CChain, CBlockIndex, CCoinsView, CTxMemPool, and similar
classes).
Code in src/node/ is meant to be segregated from code in
src/wallet/ and src/qt/, to ensure wallet and GUI
code changes don't interfere with node operation, to allow wallet and GUI code
to run in separate processes, and to perhaps eventually allow wallet and GUI
code to be maintained in separate source repositories.
As a rule of thumb, code in one of the src/node/,
src/wallet/, or src/qt/ directories should avoid
calling code in the other directories directly, and only invoke it indirectly
through the more limited src/interfaces/ classes.
This directory is at the moment
sparsely populated. Eventually more substantial files like
src/validation.cpp and
src/txmempool.cpp might be moved there.