With nId being incremented for each addr received,
an attacker could cause an overflow in the past.
(https://bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow/)
Even though that attack was made infeasible by
rate-limiting (PR bitcoin#22387), to be on the safe side change the
type to an int64_t.

…for fuzz builds

To avoid PoW being a blocker for fuzz tests,
`FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` is used in fuzz builds to
bypass the actual PoW validation in `CheckProofOfWork`. It's
replaced with a check on the last byte of the hash, which allows the
fuzzer to quickly generate (in)valid blocks by checking a single bit,
rather than performing the full PoW computation.

If PoW is the target of a fuzz test, then it should call
`CheckProofOfWorkImpl`.

…=rv32i, -mabi=ilp32)

These helpers haven't been used in production code since segwit was
merged more than eight years ago (see commit 605e847, PR bitcoin#8149),
so it seems appropriate to move them to the test utils module.

Can be reviewed via `--color-moved=dimmed-zebra`.

…rget

By reducing the number of iterations we improve the performance of
this target and may increase coverage.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

Calling `Select` with reachable networks can avoid unecessary
calls and avoid exceed the max number of tries.

When compiling with GCC 12.2, both `-Warray-bounds` and `-Wstringop-overflow` warnings were triggered in the `prevector::insert` method during CScript prevector operations.

GCC incorrectly assumed that operator new could modify the state of class members, leading to false positives during the memmove operation.

Following the approach in https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=cca06f0d6d76b0, we introduced local copies for the destination pointer in memmove operations. This prevents GCC from misinterpreting memory manipulation as unsafe.

A minimal reproducer triggering this issue in GCC 12.2 and passing in GCC 12.3 can be found at https://godbolt.org/z/8r9TKKoxv.

-------

Full error (with changes from the next commit as well):
```
In file included from /ci_container_base/src/script/script.h:11,
                 from /ci_container_base/src/primitives/transaction.h:11,
                 from /ci_container_base/src/primitives/block.h:9,
                 from /ci_container_base/src/kernel/chainparams.h:11,
                 from /ci_container_base/src/kernel/chainparams.cpp:6:
In member function ‘void prevector<N, T, Size, Diff>::fill(T*, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’,
    inlined from ‘void prevector<N, T, Size, Diff>::insert(iterator, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’ at /ci_container_base/src/prevector.h:395:13,
    inlined from ‘void CScript::AppendData(const prevector<28, unsigned char>::value_type*, size_t)’ at /ci_container_base/src/script/script.h:439:15,
    inlined from ‘CScript& CScript::operator<<(std::span<const std::byte>)’ at /ci_container_base/src/script/script.h:496:17,
    inlined from ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’ at /ci_container_base/src/kernel/chainparams.cpp:76:54:
/ci_container_base/src/prevector.h:216:13: error: writing 65 bytes into a region of size 32 [-Werror=stringop-overflow=]
  216 |             new(static_cast<void*>(dst)) T(*first);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/kernel/chainparams.cpp: In function ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’:
/ci_container_base/src/kernel/chainparams.cpp:76:49: note: destination object ‘<anonymous>’ of size 32
   76 |     const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
      |                                                 ^
In file included from /usr/lib/gcc/x86_64-w64-mingw32/12-posix/include/c++/cstring:42,
                 from /ci_container_base/src/crypto/common.h:11,
                 from /ci_container_base/src/uint256.h:9,
                 from /ci_container_base/src/consensus/params.h:9,
                 from /ci_container_base/src/kernel/chainparams.h:9:
In function ‘void* memmove(void*, const void*, size_t)’,
    inlined from ‘void prevector<N, T, Size, Diff>::insert(iterator, InputIterator, InputIterator) [with InputIterator = const unsigned char*; unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’ at /ci_container_base/src/prevector.h:393:16,
    inlined from ‘void CScript::AppendData(const prevector<28, unsigned char>::value_type*, size_t)’ at /ci_container_base/src/script/script.h:439:15,
    inlined from ‘CScript& CScript::operator<<(std::span<const std::byte>)’ at /ci_container_base/src/script/script.h:496:17,
    inlined from ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’ at /ci_container_base/src/kernel/chainparams.cpp:76:54:
/usr/share/mingw-w64/include/string.h:214:33: warning: ‘void* __builtin_memmove(void*, const void*, long long unsigned int)’ offset [65, 35] is out of the bounds [0, 32] of object ‘<anonymous>’ with type ‘CScript’ [-Warray-bounds]
  214 |   return __builtin___memmove_chk(__dst, __src, __n, __mingw_bos(__dst, 0));
      |          ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/kernel/chainparams.cpp: In function ‘CBlock CreateGenesisBlock(uint32_t, uint32_t, uint32_t, int32_t, const CAmount&)’:
/ci_container_base/src/kernel/chainparams.cpp:76:49: note: ‘<anonymous>’ declared here
   76 |     const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
      |                                                 ^
```

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

…e unused code

caac06f streams: reorder/document functions (Pieter Wuille)
67a3d59 streams: remove unused code (Pieter Wuille)

Pull request description:

  This is a follow-up to bitcoin#30884.

  Remove a number of dead code paths, and improve the code organization and documentation, in `AutoFile`.

ACKs for top commit:
  maflcko:
    re-ACK caac06f
  theStack:
    Code-review ACK caac06f
  l0rinc:
    ACK caac06f
  tdb3:
    CR ACK caac06f

Tree-SHA512: 297791f093e0142730f815c11dd3466b98f7e7edea86094a815dae989ef40d8056db10e0fed6e575d530903c18e80c08d36d3f1e6b828f2d955528f365b22008

…d-info.h

Now that this file is not in a subfolder anymore, prefix it with
"bitcoin-" to avoid potential collisions. Also add "info" for a more
descriptive name.

bitcoin-build-info.h should always be generated before clientversion.cpp
is compiled due to the following explicit dependency in src/CMakeLists.txt:

add_dependencies(bitcoin_clientversion generate_build_info)

Hence there is no need to gate the inclusion of that header with an
extra define.

7025942 build: drop superfluous `HAVE_BUILD_INFO` define (Sebastian Falbesoner)
0dd6625 build: drop obj/ subdir for generated build.h, rename to bitcoin-build-info.h (Sebastian Falbesoner)

Pull request description:

  As indicated by the TODO, the obj subdirectory is not needed anymore now for the generated build.h header, since autotools are gone and we don't have in-source builds anymore (see bitcoin#30454, bitcoin#30664). In the second commit the superflous `HAVE_BUILD_INFO` macro is dropped, as suggested in bitcoin#30856 (review).

ACKs for top commit:
  theuni:
    utACK 7025942

Tree-SHA512: 0a3b2cbbcf638344ceb74e5ba5a0fe2b1718427b23a18c8890258db36ce7177006a146178ed88d9c5ae956a5426f3844e86c1f4cca7c40946359742bffda983b

…tils

58499b0 refactor: move `SignSignature` helpers to test utils (Sebastian Falbesoner)

Pull request description:

  These helpers haven't been used in production code since segwit was merged more than eight years ago (see commit 605e847, PR bitcoin#8149), so it seems appropriate to move them to the test utils module. As suggested by instagibbs, see bitcoin#30352 (comment).

ACKs for top commit:
  instagibbs:
     ACK 58499b0
  pablomartin4btc:
    ACK 58499b0

Tree-SHA512: a52d3b92b477246f2ceb57c3690d0229a492b65a15dae331faeae9d96e5907f7fe1176edc1530243e0f088586984fd7ba435a0a2d2f2531c04d076fdf3f4095f

This ensures that the best block is included in the backup which leads to a more consistent behavior when loading the backup.

…de commands

54227e6 rpc, cli: improve error message on multiwallet mode (pablomartin4btc)

Pull request description:

  Running a CLI command when multiple wallets are loaded and `-rpcwallet` is not specified, should return a clearer error.

  Currently in `master`:

  ```
  $ bitcoin-cli -regtest -generate 1
  error code: -19
  error message:
  Wallet file not specified (must request wallet RPC through /wallet/<filename> uri-path).
  Try adding "-rpcwallet=<filename>" option to bitcoin-cli command line.
  ```

  With this change:

  ```
  $ bitcoin-cli -regtest -generate 1
  error code: -19
  error message:
  Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path. Or for the CLI, specify the "-rpcwallet=<walletname>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded).
  ```

ACKs for top commit:
  maflcko:
    review ACK 54227e6
  achow101:
    ACK 54227e6
  furszy:
    utACK 54227e6
  mzumsande:
    Code Review ACK 54227e6
  jonatack:
    ACK 54227e6

Tree-SHA512: 51ff24f64858aa6be6adf6f20105c9f076ebea743780bf2a4399f7fe8b5239cbb1ea06d32b2ef5e850da2369abb0ef7a52c50c2b8f31f4ca90d3a486abc9b77e

51f7668 addrman: change nid_type from int to int64_t (Martin Zumsande)
051ba32 addrman, refactor: introduce user-defined type for internal nId (Martin Zumsande)

Pull request description:

  With `nIdCount` being incremented for each addr received, an attacker could cause an overflow in the past, see https://bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow/
  Even though that attack was made infeasible indirectly by addr rate-limiting (PR bitcoin#22387), to be on the safe side and prevent any regressions change the `nId`s used internally to `int64_t`.
  This is being done by first introducing a user-defined type for `nId`s in the first commit, and then updating it to `int64_t` (thanks sipa for help with this!).

  Note that `nId` is only used internally, it is not part of the serialization, so `peers.dat` should not be affected by this.

  I assume that the only reason this was not done in the past is to not draw attention to this previously undisclosed issue.

ACKs for top commit:
  naumenkogs:
    ACK 51f7668
  stratospher:
    ACK 51f7668. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct.
  achow101:
    ACK 51f7668

Tree-SHA512: 68d4b8b0269a01a9544bedfa7c1348ffde00a288537e4c8bf2b88372ac7d96c4566a44dd6b06285f2fcf31b4f9336761e3bca7253fbc20db5e0d04e887156224

8466329 chain: simplify `deleteRwSettings` code and improve it's doc (ismaelsadeeq)
f8d91f4 chain: dont check for null settings value in `overwriteRwSetting` (ismaelsadeeq)
df60199 chain: ensure `updateRwSetting` doesn't update to a null settings (ismaelsadeeq)
c8e2eee chain: uniformly use `SettingsAction` enum in settings methods (ismaelsadeeq)
1e9e735 chain: move new settings safely in `overwriteRwSetting` (ismaelsadeeq)
1c40900 test: remove wallet context from `write_wallet_settings_concurrently` (ismaelsadeeq)

Pull request description:

  This PR addresses the remaining review comments from bitcoin#30697

  1. Disallowed overwriting settings values with a `null` value.
  2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
  3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
  4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.

ACKs for top commit:
  achow101:
    ACK 8466329
  ryanofsky:
    Code review ACK 8466329. Looks good, thanks for taking suggestions and applying them to the right commits. Only changes since last review were documentation improvements and simplifying delete method.
  furszy:
    Code review ACK 8466329

Tree-SHA512: baf2f59ed5aac4a4bda0c84fb6554a466a40d1f7b52b61dc2ff293d83ae60e82b925b7003237b633fecb65eba3a4c108e69166046895d1295809fbe0de67b052

e6994ef fix: increase rpcbind check robustness (tdb3)
d38e3ae fix: handle invalid rpcbind port earlier (tdb3)
83b67f2 refactor: move host/port checking (tdb3)
73c2439 test: add tests for invalid rpcbind ports (tdb3)

Pull request description:

  Previously, when an invalid port was specified in `-rpcbind`, the `SplitHostPort()` return value in `HTTPBindAddresses()` was ignored and attempt would be made to bind to the default rpcbind port (with the host/port treated as a host).

  This rearranges port checking code in `AppInitMain()` to handle the invalid port before reaching `HTTPBindAddresses()`.  Also adds a check in `HTTPBindAddresses()` as a defensive measure for future changes.

  Adds then updates associated functional tests as well.

ACKs for top commit:
  achow101:
    ACK e6994ef
  ryanofsky:
    Code review ACK e6994ef
  zaidmstrr:
    Code review ACK [e6994ef](bitcoin@e6994ef)

Tree-SHA512: bcc3e5ceef21963821cd16ce6ecb83d5c5657755882c05872a7cfe661a1492b1d631f54de22f41fdd173512d62dd15dc37e394fe1a7abe4de484b82cd2438b92

… risczero compile

bc52cda fix use int32_t instead of int type for risczero compile with (-march=rv32i, -mabi=ilp32) (Simon)

Pull request description:

  When compile bitcoin by the toolchain(`riscv32-unknown-elf-g++`) from risc0 , the compiler argument is `-march=rv32i, -mabi=ilp32`, which will get the error which due to not serialize the value of type int .

  ```
  blockbody-guest:   cargo:warning=In file included from depend/bitcoin/src/hash.h:14,
  blockbody-guest:   cargo:warning=                 from depend/bitcoin/src/script/interpreter.h:9,
  blockbody-guest:   cargo:warning=                 from depend/bitcoin/src/script/interpreter.cpp:6:
  blockbody-guest:   cargo:warning=depend/bitcoin/src/serialize.h: In instantiation of 'void Serialize(Stream&, const T&) [with Stream = HashWriter; T = int]':
  blockbody-guest:   cargo:warning=depend/bitcoin/src/hash.h:144:20:   required from 'HashWriter& HashWriter::operator<<(const T&) [with T = int]'
  blockbody-guest:   cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1613:12:   required from 'uint256 SignatureHash(const CScript&, const T&, unsigned int, int, const CAmount&, SigVersion, const PrecomputedTransactionData*) [with T = CTransaction; CAmount = long long int]'
  blockbody-guest:   cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1664:36:   required from 'bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vector<unsigned char>&, const std::vector<unsigned char>&, const CScript&, SigVersion) const [with T = CTransaction]'
  blockbody-guest:   cargo:warning=depend/bitcoin/src/script/interpreter.cpp:1785:16:   required from here
  blockbody-guest:   cargo:warning=depend/bitcoin/src/serialize.h:776:7: error: request for member 'Serialize' in 'a', which is of non-class type 'const int'
  blockbody-guest:   cargo:warning=  776 |     a.Serialize(os);
  ```

  --------------

  ### Reason

  "The toolchain from RISC Zero defines int and int32_t as different types, although they have the same width. This means that `src/compat/assumptions.h` compiles fine; however, the templated serialization code cannot accept values of type int. Fix the compilation on RISC Zero by serializing int32_t instead of int values.

  This patch will explicitly use the `int32_t` type instead of `int` to avoid errors when compiling with the risc0 toolchain. Additionally, this patch will not change any behavior on platforms where compilation was previously successful.

  Fixes bitcoin#30747

ACKs for top commit:
  maflcko:
    review-only ACK bc52cda
  achow101:
    ACK bc52cda
  TheCharlatan:
    ACK bc52cda

Tree-SHA512: ef880e7dfa1335bf2704ab17c0f506f17390b8259755674dfcd57131736492b2f4cfc36babda6902202b7c55a7513991e21f6634b0cd9b2b03baf4f1c0f8d78b

…dchacha20poly1305` target

f482d0e fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target (brunoerg)

Pull request description:

  By reducing the number of iterations we improve the performance of this target and may increase coverage.

  Running with `-runs=100000` from qa-assets I noticed a significant performance improvement and an increase on cov:
  master:
  ```
  #100000 DONE   cov: 567 ft: 4078 corp: 124/33Kb lim: 4096 exec/s: 793 rss: 499Mb
  ```

  PR:
  ```
  #100000 DONE   cov: 568 ft: 3833 corp: 113/15188b lim: 1746 exec/s: 1250 rss: 544Mb
  ```

ACKs for top commit:
  achow101:
    ACK f482d0e
  marcofleon:
    Tested ACK f482d0e. Saw the same slight increase in coverage. Executed 100,000 runs several times and total time went from 30-35 sec to 20-25 sec.
  stratospher:
    ACK f482d0e. saw similar coverage stats

Tree-SHA512: 1a96dbc22a0aed396b7f8cc9b13534b7f20a461f64f167c69c650529d535e360499f1a501abc1f957f7541ee1860b36a5580aa488a1edbfa0270c9ed83ef741d

…ain work never exceeds minimum work

284bd17 add check that chainwork doesn't exceed minimum work (marcofleon)
9aa5d1c add clarification in comment (marcofleon)

Pull request description:

  A followup to bitcoin#30661

  The added assertion just makes sure that the fuzz test is working as intended. If we're sure that the total work of the test chain is never more than minimum chain work, then we can be sure that the later assertion failure would actually mean that a bug in the headers presync logic was found.

  This PR also addresses:
  bitcoin#30661 (comment)
  bitcoin#30661 (comment)
  bitcoin#30661 (comment)

ACKs for top commit:
  instagibbs:
    reACK 284bd17
  maflcko:
    review ACK 284bd17
  achow101:
    ACK 284bd17

Tree-SHA512: 76a9dffea4b6e13499c636d6ad26af06135319d25117c0eb40cf8dfcfdca6a4549c9b4d2ba835192ca355e0f8d476227aeabf8bdb68770def72a9fb521533fe5

…pt spans, not just vectors

5e190cd Replace CScript _hex_v_u8 appends with _hex (Lőrinc)
cac846c Allow CScript's operator<< to accept spans, not just vectors (Lőrinc)
c78d8ff prevector: avoid GCC bogus warnings in insert method (Lőrinc)

Pull request description:

  Split out of bitcoin#30377 (comment).

  Replace `_hex_v_u8` for `CScript` appends to `_hex`, to skip vector conversion before serializing to the `prevector` in `CScript`.

  To enable both `unsigned char` and `std::byte` values, I've extracted the existing serialization to append the size & data in separate private methods to clarify that it does more than just a simple data insertion.

  There were also discussion on eliminating the operators here completely to obviate when we're serializing fixed-size collections as raw bytes, and when we're prefixing them with their size - should also be done in a separate PR.

ACKs for top commit:
  achow101:
    ACK 5e190cd
  ryanofsky:
    Code review ACK 5e190cd. Looks good!
  hodlinator:
    re-ACK 5e190cd

Tree-SHA512: 27a646629e017b2a05416d5eb964dda8b25b900d466221eff7bfa1339ded443e1c5c4cf8ff20cb3bba915a2603787a9fa6f6ec12bc0b9415d9eb07b57289192b

…lace RPCNotifyBlockChange, drop CRPCSignals & g_best_block

7942951 Remove unused g_best_block (Ryan Ofsky)
e3a560c rpc: use waitTipChanged for longpoll (Ryan Ofsky)
460687a Remove unused CRPCSignals (Sjors Provoost)
dca9231 Replace RPCNotifyBlockChange with waitTipChanged() (Sjors Provoost)
2a40ee1 rpc: check for negative timeout arg in waitfor* (Sjors Provoost)
de7c855 rpc: recommend -rpcclienttimeout=0 for waitfor* (Sjors Provoost)
77ec072 rpc: fix waitfornewblock description (Sjors Provoost)
285fe9f rpc: add test for waitforblock and waitfornewblock (Sjors Provoost)
b94b27c Add waitTipChanged to Mining interface (Sjors Provoost)
7eccdaf node: Track last block that received a blockTip notification (Sjors Provoost)
ebb8215 Rename getTipHash() to getTip() and return BlockRef (Sjors Provoost)
89a8f74 refactor: rename BlockKey to BlockRef (Sjors Provoost)

Pull request description:

  This continues the work in bitcoin#30200 so that a future Stratum v2 Template Provider (see bitcoin#29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients.

  `waitTipChanged()` uses a new kernel notification `notifications().m_tip_block_mutex`, which this PR also introduces (a previous version used `g_best_block`).

  In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`.

  There's a commit to add (direct) tests for the methods that are about to be refactored:
  - `waitfornewblock` was already implicitly tested by `feature_shutdown.py`.
  - `waitforblockheight` by `feature_coinstatsindex.py` and `example_test.py`

  This PR renames `getTipHash()` to `getTip()` and returns a `BlockRef` (renamed from `BlockKey`) so that callers can use either the height or hash.

  The later commits make trivial improvements to the `waitfor*` RPC calls (not needed for this PR).

  The `waitTipChanged()` method could probably also be used for the longpoll functionality in `getblocktemplate`, but I'm a bit reluctant to touch that.

  `RPCServer::OnStarted` no longer does anything and `RPCServer::OnStopped` merely prints a log statement. They were added in bitcoin#5711 as a refactor. This PR drops them entirely.

  Finally `g_best_block` is also dropped.

ACKs for top commit:
  achow101:
    ACK 7942951
  ryanofsky:
    Code review ACK 7942951. Just rebased since last review
  TheCharlatan:
    Re-ACK 7942951

Tree-SHA512: a5559446b4000c95e07aad33284b7ee2e57aafd87e1ae778b3825d59689566d047a8047e47a10f76e6e341e7dc72fd265a65afbc0a9c011d17c4cafd55031837

f20fe33 test: Add basic balance coverage to wallet_assumeutxo.py (Fabian Jahr)
037b101 test: Add coverage for best block locator write in wallet_backup (Fabian Jahr)
31c0df0 wallet: migration, write best locator before unloading wallet (furszy)
7e3dbe4 wallet: Write best block to disk before backup (Fabian Jahr)

Pull request description:

  I discovered that we don't write the best block to disk when trying to explain the behavior described here: bitcoin#30455 (comment)

  In the context of that test, the behavior is confusing and I think it also shows that one of the already existing tests in `wallet_assumeutxo.py` doesn't actually test what it says. It only fails because the best block isn't written and actually, the height of the backup that is loaded is at the snapshot height during backup. So it really shouldn't fail since it's past the background sync blocks already.

  I'm not sure if this is super relevant in practice though so I am first looking for concept ACKs on the `BackupWallet` code change. Either way, I think this behavior should be documented better if it is left as is and the test should be changed.

ACKs for top commit:
  achow101:
    ACK f20fe33
  furszy:
    ACK f20fe33

Tree-SHA512: bb384a940df5c942fffe2eb06314ade4fc5d9b924012bfef3b1c456c4182a30825d1e137d8ae561d93d3a8a2f4d1c1ffe568132d20fa7d04844f1e289ab4a28b

The Mining interface uses this function in the next commit
to calculate the coinbase merkle path. Stratum v2 uses
this to send a compact work template.

This partially undoes the change in 4defdfa,
but is not a revert, because the implementation changed in the meantime.

This commit also documents the function.