. #30411

This reverts commit 744e0e36703e26d06bc5cd1ef36a1c8568e71d05, reversing
changes made to 8209e86eeb4ceb6dd0e06c45fb3c799bb42834ab.

97e2e1d641016cd7b74848b9560e3771f092c1ea [fuzz] Use afl++ shared-memory fuzzing (dergoegge)

Pull request description:

  Using shared-memory is faster than reading from stdin, see https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md

ACKs for top commit:
  MarcoFalke:
    review ACK 97e2e1d641016cd7b74848b9560e3771f092c1ea

Tree-SHA512: 7e71b5f84835e41531c19ee959be2426da245869757de8e5dd1c730ae83ead650e2ef75f4d594d7965f661821a4ffbd27be84d3ce623702991501b34a8d02fc3

de8f9123afbecc3b4f59fa80af8148bc865d0588 test: cover read-only blockstore (Matthew Zipkin)
5c2185b3b624ce87320ec16412f98ab591a5860c ci: enable chattr +i capability inside containers (Matthew Zipkin)
e573f2420244c583e218f51cd0d3a3cac6731003 unit test: add coverage for BlockManager (Matthew Zipkin)

Pull request description:

  This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782

ACKs for top commit:
  jonatack:
    ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 modulo suggestions in https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1319010039, tested on macOS, but not on Linux for the Linux-related change in the last push
  achow101:
    ACK de8f9123afbecc3b4f59fa80af8148bc865d0588
  MarcoFalke:
    lgtm ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 📶

Tree-SHA512: b9bd684035dcea11c901b649fc39f397a2155a9a8459f3348e67947e387e45312fddeccb52981aef486f8a31deebb5356a7901c1bb94b78f82c24192a369af73

508d05f8a7b511dd53f543df8899813487eb03e5 [fuzz] Don't use afl++ deferred forkserver mode (dergoegge)

Pull request description:

  Fixes #28469

  This makes our afl++ harness essentially behave like libFuzzer, with the exception that the whole program does fully reset every 100000 iterations. 100000 is somewhat arbitrary and we could also go with `std::numeric_limits<unsigned in>::max()` but a smaller limit does allow for the occasional reset to counter act some amount of instability in the fuzzing loop (e.g. non-determinism, statefulness).

  It's a bit of a shame to do this just for the targets whose initial state can't be forked (e.g. threads) because other targets do benefit from not having to redo the state setup. An alternative would be https://github.com/bitcoin/bitcoin/issues/28469#issuecomment-1717526774:
  ```
  If the goal is to be maximally performant, the fork would need to happen for each fuzz target specifically.
  I guess it can be achieved by wrapping __AFL_INIT(); into a helper function and then require all fuzz
  target initialize() to call it?
  ```

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 508d05f8a7b511dd53f543df8899813487eb03e5

Tree-SHA512: d9fe94e2e3198795f8fb58f67eb383531a534bcd4ec75a1f0ae6ccb5531863dbc09800bb7d77536417745c4c8bc49a4f84dcc959918b27d4997a270eeacb0e7e

3f4e1bb9ae5ee43da9503da37b9894037d613c6d tests: fix incorrect assumption in v2transport_test (Pieter Wuille)

Pull request description:

  One part of the current `v2transport_test` introduced in #28196 assumes that if a bit gets modified in a message, failure should instantly be detected after sending that message. This is not correct in case the length descriptor is modified, as that may cause the receiver to need more data first. Fix this by sending more messages until failure actually occurs.

  Discovered in https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1719934041.

ACKs for top commit:
  theStack:
    ACK 3f4e1bb9ae5ee43da9503da37b9894037d613c6d

Tree-SHA512: faa90bf91996cbaaef62d764e746cb222eaf6796316b0d0e13709e528750b7c0ef09172f7fecfe814dbb8c136c5259f65ca1ac79318e6768a0bfc4e626a63249

4a825039a509c43ba20b2cd7aab448b3be16bcc3 build: use _LIBCPP_ENABLE_DEBUG_MODE over ENABLE_ASSERTIONS (fanquake)

Pull request description:

  `_LIBCPP_ENABLE_ASSERTIONS` is deprecated, and will be removed. [See (from libc++ __config in main)](https://github.com/llvm/llvm-project/blob/b57df9fe9a1a230f277d671bfa0884bbda9fc1c5/libcxx/include/__config#L205-L209):

  > TODO(hardening): remove this in LLVM 19.
  > This is for backward compatibility -- make enabling `_LIBCPP_ENABLE_ASSERTIONS` (which predates hardening modes)
  > equivalent to setting the safe mode.
  > ifdef _LIBCPP_ENABLE_ASSERTIONS
  > warning "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_SAFE_MODE instead."

  From LLVM 17, `_LIBCPP_ENABLE_DEBUG_MODE` can be used instead, which also performs more checks than safe mode:

  > Enables the debug mode which contains all the checks from the hardened mode and additionally more expensive checks that may affect the complexity of algorithms. The debug mode is intended to be used for testing, not in production. Mutually exclusive with `_LIBCPP_ENABLE_HARDENED_MODE` and `_LIBCPP_ENABLE_SAFE_MODE`.

  See https://libcxx.llvm.org/Hardening.html.

  Related to #28476.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 4a825039a509c43ba20b2cd7aab448b3be16bcc3 🙏

Tree-SHA512: ca52603f86214e8e9350bd2b2baa44fbde0f72f1b186da7aecd8690256dff5b2be75fe89383158298a6f683bbd6ae0dff528d2ba4cc5ece1f56cfbdee0e1dc5d

fa33b2c889b90bd44f188ba5f0fafe31d7d7fad7 fuzz: Add missing PROVIDE_FUZZ_MAIN_FUNCTION guard to __AFL_FUZZ_INIT (MarcoFalke)

Pull request description:

  Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62455

ACKs for top commit:
  dergoegge:
    utACK fa33b2c889b90bd44f188ba5f0fafe31d7d7fad7

Tree-SHA512: 735926f7f94ad1c3c5dc0fc62a2ef3a85abae25f4fe1e7654c2857ce3e867667ed28da58ab36281d730d3e206a0728cb429671ea5d3ccd11519e637eb191f70d

27b636a92199d2d47db5e6049de3c924d1f634f9 ci: Reintroduce fixed "test-each-commit" job (Hennadii Stepanov)

Pull request description:

  This is a fixed version of https://github.com/bitcoin/bitcoin/pull/28279:
  > Currently, if a pull request has more than one commit, previous commits may fail to compile, or may fail the tests. This is problematic, because it breaks git-bisect, or worse.
  >
  > Fix this by adding a CI task for this.

  The new job checks at most 6 commits of a pull request, excluding the top one.

  The maximum number of tested commits is 6, which derives from the time [constrains](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes).

  For historical context, please see:
  - https://github.com/bitcoin/bitcoin/pull/28279
  - https://github.com/bitcoin/bitcoin/pull/28477
  - https://github.com/bitcoin/bitcoin/pull/28478

  **A note for reviewers:** To test scripts locally, ensure that you works with a _shallow_ copy of the repo.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 27b636a92199d2d47db5e6049de3c924d1f634f9

Tree-SHA512: 0c69ced13509fa0ed2dd6ef13f4c710d678e31b294b6318b59ab1ba899086a71b5c893aaf70e143036349329167bf8e16bdca319b2c761e2aef6222d0db1470c

8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 test: wallet, verify migration doesn't crash for an invalid script (furszy)
1de8a2372ab39386e689b27d15c4d029be239319 wallet: disallow migration of invalid or not-watched scripts (furszy)

Pull request description:

  Fixing #28057.

  The legacy wallet allows to import any raw script (#28126), without
  checking if it was valid or not. Appending it to the watch-only set.

  This causes a crash in the migration process because we are only
  expecting to find valid scripts inside the legacy spkm.

  These stored scripts internally map to `ISMINE_NO` (same as if they
  weren't stored at all..).

  So we need to check for these special case, and take into account that
  the legacy spkm could be storing invalid not watched scripts.

  Which, in code words, means `IsMineInner()` returning
  `IsMineResult::INVALID` for them.

  Note:
  To verify this, can run the test commit on top of master.
  `wallet_migration.py` will crash without the bugfix commit.

ACKs for top commit:
  achow101:
    ACK 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9

Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7

fad52baf1e9bf9d55a300922e73d3bc3169a8843 fuzz: Rework addr fuzzing (MarcoFalke)
fa5b6d29ee90911271d4304a6f39c38743a84f33 fuzz: Drop unused params from serialize helpers (MarcoFalke)

Pull request description:

  Some minor fixups to addr fuzzing

ACKs for top commit:
  dergoegge:
    utACK fad52baf1e9bf9d55a300922e73d3bc3169a8843

Tree-SHA512: 6a2b07fb1a65cf855d5e7c0a52bfcb81d46dbc5d4b3e72cef359987cbd28dbfeb2fc54f210e9737cb131b40ac5f88a90e9af284e441e0b37196121590bbaf015

28bac81a346c0b68273fa73af924f7096cb3f41d test: add functional test for getaddrmaninfo (stratospher)
c8eb8dae51039aa1938e7040001a149210e87275 rpc: Introduce getaddrmaninfo for count of addresses stored in new/tried table (stratospher)

Pull request description:

  implements https://github.com/bitcoin/bitcoin/issues/26907. split off from #26988 to keep RPC, CLI discussions separate.

  This PR introduces a new RPC `getaddrmaninfo`which returns the count of addresses in the new/tried table of a node's addrman broken down by network type. This would be useful for users who want to see the distribution of addresses from different networks across new/tried table in the addrman.

  ```jsx
  $ getaddrmaninfo

  Result:
  {                   (json object) json object with network type as keys
    "network" : {     (json object) The network (ipv4, ipv6, onion, i2p, cjdns)
      "new" : n,      (numeric) number of addresses in new table
      "tried" : n,    (numeric) number of addresses in tried table
      "total" : n     (numeric) total number of addresses in both new/tried tables from a network
    },
    ...
  }
  ```

  ### additional context from [original PR](https://github.com/bitcoin/bitcoin/pull/26988)

  1. network coverage tests were skipped because there’s a small chance that addresses from different networks could hash to the same bucket and cause count of different network addresses in the tests to fail. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137596851.
  2. #26988 uses this RPC in -addrinfo CLI. Slight preference for keeping the RPC hidden since this info will mostly be useful to only super users. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808.

ACKs for top commit:
  0xB10C:
    ACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  willcl-ark:
    reACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  achow101:
    ACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  brunoerg:
    reACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  theStack:
    Code-review ACK 28bac81a346c0b68273fa73af924f7096cb3f41d

Tree-SHA512: 346390167e1ebed7ca5c79328ea452633736aff8b7feefea77460e04d4489059334ae78a3f757f32f5fb7827b309d7186bebab3c3760b3dfb016d564a647371a

f52cb02f700b58bca921a7aa24bfeee04760262b doc: make it clear that `node` in `addnode` refers to the node's address (brunoerg)
effd1efefb53c58f0e43fec4f019a19f97795553 test: `addnode` with an invalid command should throw an error (brunoerg)
56b27b84877376ffc32b3bad09f1047b23de4ba1 rpc, refactor: clean-up `addnode` (brunoerg)

Pull request description:

  This PR:

  - Adds test coverage for an invalid `command` in `addnode`.
  - Rename `test_getaddednodeinfo` to `test_addnode_getaddednodeinfo` and its log since this function also tests `addnode` and it doesn't worth to split into 2 ones.
  - Makes it clear in docs that `node` in `addnode` refers to the node's address. It seemed a little weird for me "The node (see getpeerinfo for nodes)", it could mean a lot of things e.g. the node id.
  - Some small improv/clean-up: use `const` where possible, rename some vars, and remove the check for nullance for `command` since it's a non-optional field.

ACKs for top commit:
  achow101:
    ACK f52cb02f700b58bca921a7aa24bfeee04760262b
  jonatack:
    ACK f52cb02f700b58bca921a7aa24bfeee04760262b
  theStack:
    re-ACK f52cb02f700b58bca921a7aa24bfeee04760262b

Tree-SHA512: e4a69e58b784e233463945b4d55a401957f9fe4562c129f59216a44f44fb3221d3449ac578fb35e665ca654c6ade2e741b72c3df78040f7527229c77b6c5b82e

d05be124dbc0b24fb69d0c28ba2d6b297d243751 test: added coverage to estimatefee (kevkevin)

Pull request description:

  Added a assert for an rpc error when we try to estimate fee for the max conf_target

  Line I am adding coverage to https://github.com/bitcoin/bitcoin/blob/master/src/rpc/fees.cpp#LL71C52-L71C52

ACKs for top commit:
  MarcoFalke:
    lgtm ACK d05be124dbc0b24fb69d0c28ba2d6b297d243751

Tree-SHA512: dfab075989446e33d1a5ff1a308f1ba1b9f80cce3848fbe4231f69212ceef456a3f2b19365a42123e0397c31893fd9f1fd9973cc00cfbb324386e12ed0e6bccc

eb8f58f5e4a249d55338304099e8238896d2316d Add functional test to catch too large vsize packages (Greg Sanders)
1a579f9d01256b0b2570168496d129a8b2009b35 Handle over-sized (in virtual bytes) packages with no in-mempool ancestors (Greg Sanders)
bc013fe8e3d0bae2ab766a8248219aa3c9e344df Bugfix: Pass correct virtual size to CheckPackageLimits (Luke Dashjr)
533660c58ad5a218671a9bc1537299b1d67bb55d Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusion (Greg Sanders)

Pull request description:

  (Alternative) Minimal subset of https://github.com/bitcoin/bitcoin/pull/28345 to:

  1) Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT which accounts for additional WU necessary to not exclude default chain limit transactions that would have been accepted individually. Avoids sigops vbyte confusion.
  2) pass correct vsize to chain limit evaluations in package context
  3) stop overly-large packages that have no existing mempool ancestors (also a bugfix by itself if someone sets non-standard chain limits)

  This should fix the known issues while not blocking additional refactoring later.

ACKs for top commit:
  achow101:
    ACK eb8f58f5e4a249d55338304099e8238896d2316d
  ariard:
    Re-Code ACK eb8f58f5e
  glozow:
    reACK eb8f58f5e4a249d55338304099e8238896d2316d

Tree-SHA512: 1b5cca1a526207e25d387fcc29a776a3198c3a013dc2b35c6275b9d5a64db2476c154ebf52e3a1aed0b9924c75613f21a946577aa760de28cadf0c9c7f68dc39

43cd8029fa39e0bd4bf6fb896952952bcae16160 ci: Install Homebrew's `pkg-config` package (Hennadii Stepanov)

Pull request description:

  Some versions of macOS images lack the `pkg-config` package.

  For example, https://github.com/bitcoin/bitcoin/actions/runs/6248032071/job/16961797066:
  ```
  Runner Image
    Image: macos-13
    Version: 20230417.1
  ```

  ```
  + ./autogen.sh
  configure.ac:16: error: PKG_PROG_PKG_CONFIG macro not found. Please install pkg-config and re-run autogen.sh
  ```

  This PR makes Homebrew install the `pkg-config` package explicitly.

  Also please refer to [macOS Build Guide](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md).

ACKs for top commit:
  kevkevinpal:
    ACK [43cd802](https://github.com/bitcoin/bitcoin/pull/28513/commits/43cd8029fa39e0bd4bf6fb896952952bcae16160)
  MarcoFalke:
    lgtm ACK 43cd8029fa39e0bd4bf6fb896952952bcae16160
  RandyMcMillan:
    ACK 43cd802

Tree-SHA512: 2b934b22e5f6748634089e0525b92219484e37b2afc11e9fd4c53faf112b33ca1c8deb5b4aa36939bf5c4807e7599d4aabae6335317ecc5d4a4d562bbd7dbdf2

befb42f1462f886bf5bed562ba1dae00853cecde qt: Silence `-Wcast-function-type` warning (Hennadii Stepanov)

Pull request description:

  On Fedora 38 @ 8f7b9eb8711fdb32e8bdb59d2a7462a46c7a8086:
  ```
  $ x86_64-w64-mingw32-g++ --version | head -1
  x86_64-w64-mingw32-g++ (GCC) 12.2.1 20221121 (Fedora MinGW 12.2.1-8.fc38)
  $ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site CXXFLAGS="-Wno-return-type -Wcast-function-type"
  $ make > /dev/null
  qt/winshutdownmonitor.cpp: In static member function 'static void WinShutdownMonitor::registerShutdownBlockReason(const QString&, HWND__* const&)':
  qt/winshutdownmonitor.cpp:46:42: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'PSHUTDOWNBRCREATE' {aka 'int (*)(HWND__*, const wchar_t*)'} [-Wcast-function-type]
     46 |     PSHUTDOWNBRCREATE shutdownBRCreate = (PSHUTDOWNBRCREATE)GetProcAddress(GetModuleHandleA("User32.dll"), "ShutdownBlockReasonCreate");
        |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ```

  [Required](https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1713999563) for https://github.com/bitcoin/bitcoin/pull/25972.

  Picked from https://trac.nginx.org/nginx/ticket/1865.

ACKs for top commit:
  MarcoFalke:
    review ACK befb42f1462f886bf5bed562ba1dae00853cecde

Tree-SHA512: b37b2c5dd8702caf84d1833c3511bc46ee14f23b84678b8de0fd04e24e5ecc5fd4d27ba38be0d0b08de91299369f70d4924c895a71fd8e0b6feffcfb7407574a

9ea31eba04ff8dcb9d7d993ce28bb10731a35177 gui: Disable and uncheck blank when private keys are disabled (Andrew Chow)

Pull request description:

  Unify the GUI's create wallet with the RPC createwallet so that the blank flag is not set when private keys are disabled.

ACKs for top commit:
  S3RK:
    Code review ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
  jarolrod:
    ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
  pablomartin4btc:
    tACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177

Tree-SHA512: 0c90dbd77e66f088c6a57711a4b91e254814c4ee301ab703807f281cacd4b08712d2dfeac7661f28bc0e93acc55d486a17b8b4a53ffa57093d992e7a3c51f4e8

a99e9e655a58b2364a74aec5cafb827a73c6b0c4 doc: add release note (ismaelsadeeq)
2b4edf889a4b555c8c7f6793fa5d820e5513ecac test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)
c405207a18fdee75a4dea470bb0d13e59e15ce45 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)

Pull request description:

  Coming from [#28414 comment](https://github.com/bitcoin/bitcoin/pull/28414#pullrequestreview-1618684391) Same thing also for `descriptorprocesspsbt`.

  Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction.

  In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`.

  This save users calling `finalizepsbt` with the descriptor, if it is already complete.

ACKs for top commit:
  achow101:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4
  pinheadmz:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4
  ishaanam:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4

Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75

62ab3e98ff9f9d708615dc1ef3ec04ae27f89d00 ci, windows: Do not run extended functional tests for pull requests (Hennadii Stepanov)

Pull request description:

  This PR is intended to speed up the CI feedback for pull requests:

  - a [PR](https://github.com/bitcoin/bitcoin/actions/runs/6019964104?pr=28196) opened against the current master branch:
  ![image](https://github.com/bitcoin/bitcoin/assets/32963518/481a70eb-13f3-40c9-8f6a-ca2f06350158)

  - this PR:
  ![image](https://github.com/bitcoin/bitcoin/assets/32963518/2582307f-7b72-4816-b5be-e84d5e4a3016)

  Suggested in https://github.com/bitcoin/bitcoin/pull/28173#discussion_r1302929493:

  > An alternative would be to run them on non-pr pushes only. Failures should be rare enough to deal with them post-merge.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 62ab3e98ff9f9d708615dc1ef3ec04ae27f89d00 if https://github.com/hebasto/bitcoin/actions/runs/6023862001/job/16341417883 is green

Tree-SHA512: e937efc5c03290f5d246ce1b0638dc72f39ef1d509ba5d2063f92bfe9157c602e6a952a9558dfc6413bbc5209fe55280b3278a0e4079773b8cc9ff236c8f9246

f2d4e510b37c0b132732ede214d29a8bf1daea93 ci: Avoid saving the same Ccache cache (Hennadii Stepanov)
14e5de6d0253448f9b284f0705e4697b479a8677 ci: Avoid oversubscription in functional tests on Windows (Hennadii Stepanov)

Pull request description:

  This PR aims to reduce the frequency of functional test failures on Windows like this [one](https://github.com/bitcoin/bitcoin/actions/runs/6040229997):
  ```

  2023-09-01T01:05:01.850000Z TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 552, in start_nodes
      node.wait_for_rpc_connection()
    File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 296, in wait_for_rpc_connection
      self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
    File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 177, in _raise_assertion_error
      raise AssertionError(self._node_msg(msg))
  AssertionError: [node 1] Unable to connect to bitcoind after 2400s
  ```

  This code has had zero failures in my personal repository in more than 25 runs (and is still counting).

  ---

  The second commit is a minor improvement to avoid "Cache save failed." warnings during job re-runs. For [example](https://github.com/bitcoin/bitcoin/actions/runs/5998688759):

  ![image](https://github.com/bitcoin/bitcoin/assets/32963518/d8a049df-fccd-4395-99c9-4be01d0ea706)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK f2d4e510b37c0b132732ede214d29a8bf1daea93 🐾

Tree-SHA512: 0c92817d37325a114886900e49a4d644201397d98d6ac9f2dcd41170c7e7ea2cb1873f7e51b5cb3ad3cc2e59554ad1c8f87d439ea6c1c960bf5c339153be7040

fa33b2c889b90bd44f188ba5f0fafe31d7d7fad7 fuzz: Add missing PROVIDE_FUZZ_MAIN_FUNCTION guard to __AFL_FUZZ_INIT (MarcoFalke)

Pull request description:

  Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62455

ACKs for top commit:
  dergoegge:
    utACK fa33b2c889b90bd44f188ba5f0fafe31d7d7fad7

Tree-SHA512: 735926f7f94ad1c3c5dc0fc62a2ef3a85abae25f4fe1e7654c2857ce3e867667ed28da58ab36281d730d3e206a0728cb429671ea5d3ccd11519e637eb191f70d

27b636a92199d2d47db5e6049de3c924d1f634f9 ci: Reintroduce fixed "test-each-commit" job (Hennadii Stepanov)

Pull request description:

  This is a fixed version of https://github.com/bitcoin/bitcoin/pull/28279:
  > Currently, if a pull request has more than one commit, previous commits may fail to compile, or may fail the tests. This is problematic, because it breaks git-bisect, or worse.
  >
  > Fix this by adding a CI task for this.

  The new job checks at most 6 commits of a pull request, excluding the top one.

  The maximum number of tested commits is 6, which derives from the time [constrains](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes).

  For historical context, please see:
  - https://github.com/bitcoin/bitcoin/pull/28279
  - https://github.com/bitcoin/bitcoin/pull/28477
  - https://github.com/bitcoin/bitcoin/pull/28478

  **A note for reviewers:** To test scripts locally, ensure that you works with a _shallow_ copy of the repo.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 27b636a92199d2d47db5e6049de3c924d1f634f9

Tree-SHA512: 0c69ced13509fa0ed2dd6ef13f4c710d678e31b294b6318b59ab1ba899086a71b5c893aaf70e143036349329167bf8e16bdca319b2c761e2aef6222d0db1470c

8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 test: wallet, verify migration doesn't crash for an invalid script (furszy)
1de8a2372ab39386e689b27d15c4d029be239319 wallet: disallow migration of invalid or not-watched scripts (furszy)

Pull request description:

  Fixing #28057.

  The legacy wallet allows to import any raw script (#28126), without
  checking if it was valid or not. Appending it to the watch-only set.

  This causes a crash in the migration process because we are only
  expecting to find valid scripts inside the legacy spkm.

  These stored scripts internally map to `ISMINE_NO` (same as if they
  weren't stored at all..).

  So we need to check for these special case, and take into account that
  the legacy spkm could be storing invalid not watched scripts.

  Which, in code words, means `IsMineInner()` returning
  `IsMineResult::INVALID` for them.

  Note:
  To verify this, can run the test commit on top of master.
  `wallet_migration.py` will crash without the bugfix commit.

ACKs for top commit:
  achow101:
    ACK 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9

Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7

fad52baf1e9bf9d55a300922e73d3bc3169a8843 fuzz: Rework addr fuzzing (MarcoFalke)
fa5b6d29ee90911271d4304a6f39c38743a84f33 fuzz: Drop unused params from serialize helpers (MarcoFalke)

Pull request description:

  Some minor fixups to addr fuzzing

ACKs for top commit:
  dergoegge:
    utACK fad52baf1e9bf9d55a300922e73d3bc3169a8843

Tree-SHA512: 6a2b07fb1a65cf855d5e7c0a52bfcb81d46dbc5d4b3e72cef359987cbd28dbfeb2fc54f210e9737cb131b40ac5f88a90e9af284e441e0b37196121590bbaf015

28bac81a346c0b68273fa73af924f7096cb3f41d test: add functional test for getaddrmaninfo (stratospher)
c8eb8dae51039aa1938e7040001a149210e87275 rpc: Introduce getaddrmaninfo for count of addresses stored in new/tried table (stratospher)

Pull request description:

  implements https://github.com/bitcoin/bitcoin/issues/26907. split off from #26988 to keep RPC, CLI discussions separate.

  This PR introduces a new RPC `getaddrmaninfo`which returns the count of addresses in the new/tried table of a node's addrman broken down by network type. This would be useful for users who want to see the distribution of addresses from different networks across new/tried table in the addrman.

  ```jsx
  $ getaddrmaninfo

  Result:
  {                   (json object) json object with network type as keys
    "network" : {     (json object) The network (ipv4, ipv6, onion, i2p, cjdns)
      "new" : n,      (numeric) number of addresses in new table
      "tried" : n,    (numeric) number of addresses in tried table
      "total" : n     (numeric) total number of addresses in both new/tried tables from a network
    },
    ...
  }
  ```

  ### additional context from [original PR](https://github.com/bitcoin/bitcoin/pull/26988)

  1. network coverage tests were skipped because there’s a small chance that addresses from different networks could hash to the same bucket and cause count of different network addresses in the tests to fail. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137596851.
  2. #26988 uses this RPC in -addrinfo CLI. Slight preference for keeping the RPC hidden since this info will mostly be useful to only super users. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808.

ACKs for top commit:
  0xB10C:
    ACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  willcl-ark:
    reACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  achow101:
    ACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  brunoerg:
    reACK 28bac81a346c0b68273fa73af924f7096cb3f41d
  theStack:
    Code-review ACK 28bac81a346c0b68273fa73af924f7096cb3f41d

Tree-SHA512: 346390167e1ebed7ca5c79328ea452633736aff8b7feefea77460e04d4489059334ae78a3f757f32f5fb7827b309d7186bebab3c3760b3dfb016d564a647371a

f52cb02f700b58bca921a7aa24bfeee04760262b doc: make it clear that `node` in `addnode` refers to the node's address (brunoerg)
effd1efefb53c58f0e43fec4f019a19f97795553 test: `addnode` with an invalid command should throw an error (brunoerg)
56b27b84877376ffc32b3bad09f1047b23de4ba1 rpc, refactor: clean-up `addnode` (brunoerg)

Pull request description:

  This PR:

  - Adds test coverage for an invalid `command` in `addnode`.
  - Rename `test_getaddednodeinfo` to `test_addnode_getaddednodeinfo` and its log since this function also tests `addnode` and it doesn't worth to split into 2 ones.
  - Makes it clear in docs that `node` in `addnode` refers to the node's address. It seemed a little weird for me "The node (see getpeerinfo for nodes)", it could mean a lot of things e.g. the node id.
  - Some small improv/clean-up: use `const` where possible, rename some vars, and remove the check for nullance for `command` since it's a non-optional field.

ACKs for top commit:
  achow101:
    ACK f52cb02f700b58bca921a7aa24bfeee04760262b
  jonatack:
    ACK f52cb02f700b58bca921a7aa24bfeee04760262b
  theStack:
    re-ACK f52cb02f700b58bca921a7aa24bfeee04760262b

Tree-SHA512: e4a69e58b784e233463945b4d55a401957f9fe4562c129f59216a44f44fb3221d3449ac578fb35e665ca654c6ade2e741b72c3df78040f7527229c77b6c5b82e

d05be124dbc0b24fb69d0c28ba2d6b297d243751 test: added coverage to estimatefee (kevkevin)

Pull request description:

  Added a assert for an rpc error when we try to estimate fee for the max conf_target

  Line I am adding coverage to https://github.com/bitcoin/bitcoin/blob/master/src/rpc/fees.cpp#LL71C52-L71C52

ACKs for top commit:
  MarcoFalke:
    lgtm ACK d05be124dbc0b24fb69d0c28ba2d6b297d243751

Tree-SHA512: dfab075989446e33d1a5ff1a308f1ba1b9f80cce3848fbe4231f69212ceef456a3f2b19365a42123e0397c31893fd9f1fd9973cc00cfbb324386e12ed0e6bccc

eb8f58f5e4a249d55338304099e8238896d2316d Add functional test to catch too large vsize packages (Greg Sanders)
1a579f9d01256b0b2570168496d129a8b2009b35 Handle over-sized (in virtual bytes) packages with no in-mempool ancestors (Greg Sanders)
bc013fe8e3d0bae2ab766a8248219aa3c9e344df Bugfix: Pass correct virtual size to CheckPackageLimits (Luke Dashjr)
533660c58ad5a218671a9bc1537299b1d67bb55d Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusion (Greg Sanders)

Pull request description:

  (Alternative) Minimal subset of https://github.com/bitcoin/bitcoin/pull/28345 to:

  1) Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT which accounts for additional WU necessary to not exclude default chain limit transactions that would have been accepted individually. Avoids sigops vbyte confusion.
  2) pass correct vsize to chain limit evaluations in package context
  3) stop overly-large packages that have no existing mempool ancestors (also a bugfix by itself if someone sets non-standard chain limits)

  This should fix the known issues while not blocking additional refactoring later.

ACKs for top commit:
  achow101:
    ACK eb8f58f5e4a249d55338304099e8238896d2316d
  ariard:
    Re-Code ACK eb8f58f5e
  glozow:
    reACK eb8f58f5e4a249d55338304099e8238896d2316d

Tree-SHA512: 1b5cca1a526207e25d387fcc29a776a3198c3a013dc2b35c6275b9d5a64db2476c154ebf52e3a1aed0b9924c75613f21a946577aa760de28cadf0c9c7f68dc39

43cd8029fa39e0bd4bf6fb896952952bcae16160 ci: Install Homebrew's `pkg-config` package (Hennadii Stepanov)

Pull request description:

  Some versions of macOS images lack the `pkg-config` package.

  For example, https://github.com/bitcoin/bitcoin/actions/runs/6248032071/job/16961797066:
  ```
  Runner Image
    Image: macos-13
    Version: 20230417.1
  ```

  ```
  + ./autogen.sh
  configure.ac:16: error: PKG_PROG_PKG_CONFIG macro not found. Please install pkg-config and re-run autogen.sh
  ```

  This PR makes Homebrew install the `pkg-config` package explicitly.

  Also please refer to [macOS Build Guide](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md).

ACKs for top commit:
  kevkevinpal:
    ACK [43cd802](https://github.com/bitcoin/bitcoin/pull/28513/commits/43cd8029fa39e0bd4bf6fb896952952bcae16160)
  MarcoFalke:
    lgtm ACK 43cd8029fa39e0bd4bf6fb896952952bcae16160
  RandyMcMillan:
    ACK 43cd802

Tree-SHA512: 2b934b22e5f6748634089e0525b92219484e37b2afc11e9fd4c53faf112b33ca1c8deb5b4aa36939bf5c4807e7599d4aabae6335317ecc5d4a4d562bbd7dbdf2

befb42f1462f886bf5bed562ba1dae00853cecde qt: Silence `-Wcast-function-type` warning (Hennadii Stepanov)

Pull request description:

  On Fedora 38 @ 8f7b9eb8711fdb32e8bdb59d2a7462a46c7a8086:
  ```
  $ x86_64-w64-mingw32-g++ --version | head -1
  x86_64-w64-mingw32-g++ (GCC) 12.2.1 20221121 (Fedora MinGW 12.2.1-8.fc38)
  $ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site CXXFLAGS="-Wno-return-type -Wcast-function-type"
  $ make > /dev/null
  qt/winshutdownmonitor.cpp: In static member function 'static void WinShutdownMonitor::registerShutdownBlockReason(const QString&, HWND__* const&)':
  qt/winshutdownmonitor.cpp:46:42: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'PSHUTDOWNBRCREATE' {aka 'int (*)(HWND__*, const wchar_t*)'} [-Wcast-function-type]
     46 |     PSHUTDOWNBRCREATE shutdownBRCreate = (PSHUTDOWNBRCREATE)GetProcAddress(GetModuleHandleA("User32.dll"), "ShutdownBlockReasonCreate");
        |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ```

  [Required](https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1713999563) for https://github.com/bitcoin/bitcoin/pull/25972.

  Picked from https://trac.nginx.org/nginx/ticket/1865.

ACKs for top commit:
  MarcoFalke:
    review ACK befb42f1462f886bf5bed562ba1dae00853cecde

Tree-SHA512: b37b2c5dd8702caf84d1833c3511bc46ee14f23b84678b8de0fd04e24e5ecc5fd4d27ba38be0d0b08de91299369f70d4924c895a71fd8e0b6feffcfb7407574a

9ea31eba04ff8dcb9d7d993ce28bb10731a35177 gui: Disable and uncheck blank when private keys are disabled (Andrew Chow)

Pull request description:

  Unify the GUI's create wallet with the RPC createwallet so that the blank flag is not set when private keys are disabled.

ACKs for top commit:
  S3RK:
    Code review ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
  jarolrod:
    ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
  pablomartin4btc:
    tACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177

Tree-SHA512: 0c90dbd77e66f088c6a57711a4b91e254814c4ee301ab703807f281cacd4b08712d2dfeac7661f28bc0e93acc55d486a17b8b4a53ffa57093d992e7a3c51f4e8

a99e9e655a58b2364a74aec5cafb827a73c6b0c4 doc: add release note (ismaelsadeeq)
2b4edf889a4b555c8c7f6793fa5d820e5513ecac test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)
c405207a18fdee75a4dea470bb0d13e59e15ce45 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)

Pull request description:

  Coming from [#28414 comment](https://github.com/bitcoin/bitcoin/pull/28414#pullrequestreview-1618684391) Same thing also for `descriptorprocesspsbt`.

  Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction.

  In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`.

  This save users calling `finalizepsbt` with the descriptor, if it is already complete.

ACKs for top commit:
  achow101:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4
  pinheadmz:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4
  ishaanam:
    ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4

Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75

this will break bip324 tests but i think is needed because we have up to 100mb message size max and 3 bytes can only encrypt up to 16mb

uses void ptr, maybe we can think of a better way

b3db8c9d5ccfe5c31341169fa7ac044427122921 rpc: bumpfee, improve doc for 'reduce_output' arg (furszy)

Pull request description:

  Fixes #28180. Resulted from discussions with S3RK, achow101, and Murch.

  The current argument name and description are dangerous as it don't
  describe the case where the user selects the recipient output as the
  change address. This one could end up been increased by the inputs
  minus outputs remainder. Which, when `bumpfee` adds new inputs
  to the transaction, leads the process to send more coins to the
  recipient. Which is not what the user would expect from a
  'reduce_output' param naming.

ACKs for top commit:
  S3RK:
    ACK b3db8c9d5ccfe5c31341169fa7ac044427122921
  achow101:
    ACK b3db8c9d5ccfe5c31341169fa7ac044427122921
  murchandamus:
    ACK b3db8c9d5ccfe5c31341169fa7ac044427122921

Tree-SHA512: 91f607e2f5849041d7c099afdddae11af8bed5b1ac90c9d22921267f272e21b44e107d6968e037f05f958a61fe29e94e5fb44b224fb3606f197f83ec4ba3b1e7

262ab8ef7860d43cebc9d04721e3a075b4edf06e Add package evaluation fuzzer (Greg Sanders)

Pull request description:

  This fuzzer target caught the issue in https://github.com/bitcoin/bitcoin/pull/28251 within 5 minutes on master branch, and an additional issue which I've applied a preliminary patch to cover.

  Fuzzer target does the following:

  1) Picks mempool confgs, including max package size, count, mempool size, etc
  2) Generates 1 to 26 transactions with arbitrary coins/fees, the first N-1 spending only confirmed outpoints
  3) Nth transaction, if >1, sweeps all unconfirmed outpoints in mempool
  4) If N==1, it may submit it through single-tx submission path, to allow for more interesting topologies
  5) Otherwise submits through package submission interface
  6) Repeat 1-5  a few hundred times per mempool instance

  In other words, it ends up building chains of txns in the mempool using parents-and-children packages, which is currently the topology supported on master.

  The test itself is a direct rip of tx_pool.cpp, with a number of assertions removed because they were failing for unknown reasons, likely due to the notification changes of single tx submission to package, which is used to track addition/removal of transactions in the test. I'll continue working on re-adding these assertions for further invariant testing.

ACKs for top commit:
  murchandamus:
    ACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e
  glozow:
    reACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e
  dergoegge:
    tACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e

Tree-SHA512: 190784777d0f2361b051b3271db8f79b7927e3cab88596d2c30e556da721510bd17f6cc96f6bb03403bbf0589ad3f799fa54e63c1b2bd92a2084485b5e3e96a5

b5a962564eb15075e4e2a7bc0c235a56fa998ac3 tests: Use manual bumps instead of bumpfee for resendwallettransactions (Andrew Chow)

Pull request description:

  Bumpfee will try to increase the entire package to the target feerate, which causes repeated bumpfees to quickly shoot up in fees, causing intermittent failures when the fee is too large. We don't care about this property, just that the child is continuously replaced until we observe it's position in mapWallet is before its parent. Instead of using bumpfee, we can create raw transactions which have only pay (just above) the additional incremental relay fee, thus avoiding this problem.

  Fixes #28491

ACKs for top commit:
  kevkevinpal:
    ACK [b5a9625](https://github.com/bitcoin/bitcoin/pull/28540/commits/b5a962564eb15075e4e2a7bc0c235a56fa998ac3)
  mzumsande:
    Code review ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3
  pablomartin4btc:
    ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3 -> adding the `try_rpc` to avoid (skip) any possible failure around the manual bump fee (if we ever reach it as [explained](https://github.com/bitcoin/bitcoin/pull/28540#issuecomment-1737648048)) makes a lot of sense as the spirit of the test is the tx (child before parent) sort in the `mapWallet` (as also [explained](https://github.com/bitcoin/bitcoin/issues/28491#issuecomment-1736161363)).
  MarcoFalke:
    lgtm ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3

Tree-SHA512: f184f11c73be0c30753181901f51a3b4b9c4135e0c4681e9f4ca94692c49bac15c91683c85266a2124333c8593e9919bfd9102724616faab299740f2eb98741f

e3720bca398820038b3e97f467adb2c45ef9ef5f net: Simplify v2 recv logic by decoupling AAD from state machine (Tim Ruffing)
b0f5175c044df956c0f07f540706d457c4912856 net: Drop v2 garbage authentication packet (Tim Ruffing)

Pull request description:

  Note that this is a breaking change, see also https://github.com/bitcoin/bips/pull/1498

  The benefit is a simpler implementation:
   - The protocol state machine does not need separate states for garbage authentication and version phases.
   - The special case of "ignoring the ignore bit" is removed.
   - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing.

ACKs for top commit:
  naumenkogs:
    ACK e3720bca398820038b3e97f467adb2c45ef9ef5f
  sipa:
    ACK e3720bca398820038b3e97f467adb2c45ef9ef5f. Re-ran the v2 transport fuzzer overnight.
  ajtowns:
    ACK e3720bca398820038b3e97f467adb2c45ef9ef5f - simpler and more flexible, nice
  achow101:
    ACK e3720bca398820038b3e97f467adb2c45ef9ef5f
  Sjors:
    utACK e3720bca398820038b3e97f467adb2c45ef9ef5f
  theStack:
    Code-review ACK e3720bca398820038b3e97f467adb2c45ef9ef5f

Tree-SHA512: 16600ed868c8a346828de075c4072e37cf86440751d08ab099fe8b58ff71d8b371a90397d6a4247096796db68794275e7e0403f218859567d04838e0411dadd6

d8041d4e042957660827313951b18c8dd9a99a16 blockstorage: Return on fatal undo file flush error (TheCharlatan)
f0207e00303a1030eca795ede231e3c0d94df061 blockstorage: Return on fatal block file flush error (TheCharlatan)
5671c15f4520c6dc20e0805fd0b06157ff94bcd7 blockstorage: Mark FindBlockPos as nodiscard (TheCharlatan)

Pull request description:

  The goal of this PR is to establish that fatal blockstorage flush errors should be treated as errors at their call site.

  Prior to this patch `FlushBlockFile` may have failed without returning in `Chainstate::FlushStateToDisk`, leading to a potential write from `WriteBlockIndexDB` that may refer to a block that is not fully flushed to disk yet. By returning if either `FlushUndoFile` or `FlushBlockFile` fail, we ensure that no further write operations take place that may lead to an inconsistent database when crashing. Add `[[nodiscard]]` annotations to them such that they are not ignored in future.

  Functions that call either `FlushUndoFile` or `FlushBlockFile`, need to handle these extra abort cases properly. Since `Chainstate::FlushStateToDisk` already produces an abort error in case of `WriteBlockIndexDB` failing, no extra logic for functions calling `Chainstate::FlushStateToDisk` is required.

  Besides `Chainstate::FlushStateToDisk`, `FlushBlockFile` is also called by `FindBlockPos`, while `FlushUndoFile` is only called by `FlushBlockFile` and `WriteUndoDataForBlock`. For both these cases, the flush error is not further bubbled up. Instead, the error is logged and a comment is provided why bubbling up an error would be less desirable in these cases.

  ---

  This pull request is part of a larger effort towards improving the shutdown / abort / fatal error handling in validation code. It is a first step towards implementing proper fatal error return type enforcement similar as proposed by theuni in this pull request [comment](https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1563561502). For ease of review of these critical changes, a first step would be checking that `AbortNode` leads to early and error-conveying returns at its call site. Further work for enforcing returns when `AbortNode` is called is done in https://github.com/bitcoin/bitcoin/pull/27862.

ACKs for top commit:
  stickies-v:
    re-ACK d8041d4
  ryanofsky:
    Code review ACK d8041d4e042957660827313951b18c8dd9a99a16

Tree-SHA512: 47ade9b873b15e567c8f60ca538d5a0daf32163e1031be3212a3a45eb492b866664b225f2787c9e40f3e0c089140157d8fd1039abc00c7bdfeec1b52ecd7e219

96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983 test: add unit test coverage for Python ECDSA implementation (Sebastian Falbesoner)

Pull request description:

  This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in the test framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point.

  To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose, the dictionary storing private-key/public-key entries use their legacy types `ECKey/ECPubKey` instead of bare byte-arrays, and for Schnorr signing/verification the necessary conversions (ECKey -> bare private key, ECPubKey -> x-only pubkey) is done later when needed. To avoid code duplication, a helper function `random_bitflip` for damaging signatures is introduced.

  The unit test can be run by either calling it for this single module:
  `$ python3 -m unittest ./test/functional/test_framework/key.py`
  or simply running `$ ./test/functional/test_runner.py` which calls all test framework module's unit tests at the start (see TEST_FRAMEWORK_MODULES list).

ACKs for top commit:
  achow101:
    ACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983
  sipa:
    utACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983
  stratospher:
    tested ACK 96b3f2d.

Tree-SHA512: b993f25b843fa047376addda4ce4b0f15750ffba926528b5cca4c5f99b9af456206f4e8af885d25a017dddddf382ddebf38765819b3d16a3f28810d03b010808

fa40b3ee22e78f58d7426dbc4343472ba40081e3 test: Avoid test failure on Linux root without cap-add LINUX_IMMUTABLE (MarcoFalke)

Pull request description:

  This turns a test failure on Linux when running the test as `root`, but without the `LINUX_IMMUTABLE` capability, into an early return, with a suggestion to turn on `LINUX_IMMUTABLE` next time (if possible).

ACKs for top commit:
  pinheadmz:
    utACK fa40b3ee22e78f58d7426dbc4343472ba40081e3
  jonatack:
    ACK fa40b3ee22e78f58d7426dbc4343472ba40081e3

Tree-SHA512: d986ff8aeae5f8267c21a23d5be16f7c5a4d4d3be045a6999d8b39c7b8672cfe915dedde762cc9965cdc4970940bffc4b0d1412833d8036d4425450eb6181f67

f9047771d642c5887c752872b6ffbbd974603b35 lint: fix custom mypy cache dir setting (Fabian Jahr)

Pull request description:

  fixes #28183

  The custom cache dir for `mypy` can only be set via an environment variable, setting the `MYPY_CACHE_DIR` variable in the program is not sufficient. This error was introduced while translating the shell script to python.

  See also the mypy documentation: https://mypy.readthedocs.io/en/stable/config_file.html#confval-cache_dir

ACKs for top commit:
  MarcoFalke:
    lgtm ACK f9047771d642c5887c752872b6ffbbd974603b35

Tree-SHA512: 7e8fb0cd06688129bd46d1afb8647262eb53d0f60b1ef6f288fedaa122d906fb62c9855e8bb0d6c6297d41a87a47d3cec7a00df55a7d033947937dfe23d07ba7

2ab7952bda8d15e91b03f8307839030cbb55614e test: add bip157 coverage for (start height > stop height) disconnect (Sebastian Falbesoner)
63e90e1d3f5ed08f9871f07667d389ec66aa621c test: check for specific disconnect reasons in p2p_blockfilters.py (Sebastian Falbesoner)

Pull request description:

  This PR checks for specific disconnect reasons using `assert_debug_log` in the functional test `p2p_blockfilters.py`. With that we ensure that the disconnect happens for the expected reason and also makes it easier to navigate between implementation and test code, i.e. both the questions "do we have test coverage for this disconnect cause?" (from an implementation reader's perspective) and "where is the code handling this disconnect cause?" (from a test reader's perspective) can be answered simply by grep-ping the corresponding debug message.

  Also, based on that, missing coverage for the (start height > stop height) disconnect case is added:
  https://github.com/bitcoin/bitcoin/blob/b7138252ace6d21476964774e094ed1143cd7a1c/src/net_processing.cpp#L3050-L3056

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 2ab7952bda8d15e91b03f8307839030cbb55614e
  furszy:
    Looks good, code ACK 2ab7952b

Tree-SHA512: 0581cb569d5935aaa004a95a6f16eeafe628b9d816ebb89232f2832e377049df878a1e74c369fb46931b94e1a3a5e3f4aaa21a007c0a488f4ad2cda0919c605d

d9b172cd00fc3a8de1308e4469b82f5da474ea33 doc: fix link to developer-notes.md file in multiprocess.md (David Álvarez Rosa)

Pull request description:

  Fix link to `developer-notes.md` file in `multiprocess.md`.

ACKs for top commit:
  fanquake:
    ACK d9b172cd00fc3a8de1308e4469b82f5da474ea33

Tree-SHA512: 55fffefb37c4d67acb1fa8b0660216ec1c7f2c2314d11e4d319cae40480ed59ef448909fa2ca334167c86d60d41932220dce4186e28fa300f4d03eb0b3c769d0

782701ce7d31919dba2241ee43b582d8ae5a2541 test: Test loading wallets with conflicts without a chain (Andrew Chow)
4660fc82a1f5cf6eb6404d5268beef5919581661 wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow)

Pull request description:

  `MarkConflicted` assumes that `m_last_block_processed_height` is always valid. However it may not be valid when a chain is not attached, as happens in the wallet tool and during migration. In such situations, when the conflicting height is also negative (which occurs on loading when no chain is available), the calculation of the number of conflict confirms results in a non-negative value which passes the existing check for valid values. This will subsequently hit an assertion in `GetTxDepthInMainChain`.

  Furthermore, `MarkConflicted` is also only called on loading a transaction whose parent has a stored state of `TxStateConflicted` and was loaded before the child transaction. This depends on the loading order, which for both sqlite and bdb depends on the txids.

  We can avoid this by explicitly checking that both `m_last_block_processed_height` and `conflicting_height` are non-negative. Both `tool_wallet.py` and `wallet_migration.py` are updated to create wallets with a state that triggers the assertion.

  Fixes #28510

ACKs for top commit:
  ryanofsky:
    Code review ACK 782701ce7d31919dba2241ee43b582d8ae5a2541. Nice catch, and clever test (grinding the txid)
  furszy:
    ACK 782701ce

Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9

380130d9d70f8f8d395949a51f43806f6e600efa test: add coverage to feature_addrman.py (kevkevin)

Pull request description:

  I added two new tests that will cover the nNew and nTried tests which add coverage to the if block by checking values larger than our range since we only check for negative values now

  adding coverage to these lines
  https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L273
  https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L280

  our test seem to only cover the `nTried < 0` and `nNew < 0` scenarios

ACKs for top commit:
  ismaelsadeeq:
    ACK 380130d9d70f8f8d395949a51f43806f6e600efa, code looks good to me 🍃 .
  0xB10C:
    Re-ACK 380130d9d70f8f8d395949a51f43806f6e600efa

Tree-SHA512: a063bd9ca4d2d536a27c8c22a28fb13759a96f19cd8ba6cb8879cf7f65046d4ff6e8f70df17feaffd0d0d08ef914cb18a11258d313a4841c811a7e7ae4df6d5b

d67aa25eb2f781b9edcfcf164a08401f9937a0c1 bench: drop NO_THREAD_SAFETY_ANALYSIS from disconnected_txs (fanquake)

Pull request description:

  Followup to https://github.com/bitcoin/bitcoin/pull/28385#discussion_r1339964480.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK d67aa25eb2f781b9edcfcf164a08401f9937a0c1
  hebasto:
    ACK d67aa25eb2f781b9edcfcf164a08401f9937a0c1, tested on Ubuntu 22.04 with clang 18.0.
  glozow:
    utACK d67aa25eb2

Tree-SHA512: a9a9a8cc70a50d4fbd51779a3203bbc2a29d354b557e8a99cfd649d5998b71ff1087f5bae7170471bed9a917a93c8f3351ae90c9a6e87d88928c35912d007b64

79ef528511f0cbbe0a7097ef031f2964aaccfe5c build, macos: Fix `qt` package build with new Xcode 15 linker (Hennadii Stepanov)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/28541 by backporting an upstream [patch](https://github.com/qt/qtbase/commit/cdf64b0e47115cc473e1afd1472b4b09e130b2a5).

  Guix build:
  ```
  x86_64
  b37713bc8a526662eac3d9535924f4a4d2893c58f9c12d3c7599e761e6ff677c  guix-build-79ef528511f0/output/arm64-apple-darwin/SHA256SUMS.part
  0befb524181aa10e1635a2616a8bed53f51beafa4f0d495d3bf52a64cbd2d977  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.tar.gz
  9cba170f2ffe542c33fdd1ac52b7684dd6301e91d32aa45af7b4ce8769d88d4a  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.zip
  04556309266c791ae4d7409359222c88cd7aeb569566f7ef4d29816148a5b7e4  guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin.tar.gz
  51229df8e104a2ffcd5c5b3f81f7585e1258ef10461d136948ea2a2d690a920d  guix-build-79ef528511f0/output/dist-archive/bitcoin-79ef528511f0.tar.gz
  3fe216a05561f2fe7229ddf186ff495b29a5cc31b6f35f407187573d072c5743  guix-build-79ef528511f0/output/x86_64-apple-darwin/SHA256SUMS.part
  961d71104e61a2baf727576eb2da630697bb4f109f66e73be5c96add25378d12  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.tar.gz
  5598f514d065756ac376e2f3c4f8e758bfba53a43ddef778f106456de1536073  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.zip
  5360ae1f1b7d96a44a33b2c87708b466e4a7bf3f9de0fc58bccbbcdb21ee254e  guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin.tar.gz
  ```

Top commit has no ACKs.

Tree-SHA512: e3a0f7a578b30a216cc84c8ac6a0eeac3f59b02525e1eb5a9f5512bc9a049a1b17d3feb140259ffe5d2197279c74594126b85112aa596df9013f74bb1047c298

fa6e6a3f03a38f8b431bf694268ed344d1815b3b doc: Remove confusing assert linter (MarcoFalke)

Pull request description:

  The `assert()` documentation and linter are redundant and confusing:

  * The source code already refuses to compile with `assert()` disabled.
  * They violate the assumptions about `Assert()`, which *requires* side effects.
  * The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects.

  Fix all issues by removing the docs and the linter. See also https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1287370102

  Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit.

ACKs for top commit:
  hebasto:
    ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b, I have reviewed the code and it looks OK.
  theStack:
    ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b

Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236

bae209e3879fa099302d3b211362c49bbbfbdd14 gui: macOS, make appMenuBar part of the main app window (furszy)
e14cc8fc69cb3e3a98076fbb23a94eba7873368a gui: macOS, do not process dock icon actions during shutdown (furszy)

Pull request description:

  As the 'QMenuBar' is created without a parent window in MacOS, the app crashes when the user presses the shutdown button and, right after it, triggers any action in the menu bar.

  This happens because the QMenuBar is manually deleted in the BitcoinGUI destructor but the events attached to it children actions are not disconnected, so QActions events such us the 'QMenu::aboutToShow' could try to access null pointers.

  Instead of guarding every single QAction pointer inside the QMenu::aboutToShow slot, or manually disconnecting all registered events in the destructor, we can check if a shutdown was requested and discard the event.

  The 'node' field is a ref whose memory is held by the main application class, so it is safe to use here. Events are disconnected prior destructing the main application object.

  Furthermore, the 'MacDockIconHandler::dockIconClicked' signal can make the app crash during shutdown for the very same reason. The 'show()' call triggers the 'QApplication::focusWindowChanged' event, which is connected to the 'minimize_action' QAction, which is also part of the app menu bar, which could no longer exist.

  Another cause of crashes stems from the shortcuts provided by the `appMenuBar` submenus during shutdown. For instance, executing actions like opening the information dialog (command + I) or the console dialog (command + T) lead to access null pointers. The second commit addresses and resolves these issues.
  Basically, in the present setup, we create a parentless `appMenuBar` whose submenus `QActions` are connected to `qApp` events (the app's global instance). However, at the `BitcoinGUI` destructor, we manually destruct this object without properly disconnecting the events. This leaves `qApp` events, such as `focusWindowChanged`, tied to submenus' `QAction` pointers, which causes the application to crash when it attempts to access them.

  Important Note:
  This happened to me few times. The worst consequence was an inconsistent chain state during IBD. Which triggered a full "replay blocks" process on the next startup. Which was painfully slow.

ACKs for top commit:
  RandyMcMillan:
    utACK bae209e
  hebasto:
    ACK bae209e3879fa099302d3b211362c49bbbfbdd14.

Tree-SHA512: 432e19c5f7e02c3165b7e7bd7f96f2a902bae5b5e439c2594db1c69d74ab6e0d4509d90f02db8c076f616e567e6a07492ede416ef651b5f749637398291b92fd

7df450836969b81e98322c9a09c08b35d1095a25 test: improve sock_tests/move_assignment (Vasil Dimov)
5086a99b84367a45706af7197da1016dd966e6d9 net: remove Sock default constructor, it's not necessary (Vasil Dimov)
7829272f7826511241defd34954e6040ea963f07 net: remove now unnecessary Sock::Get() (Vasil Dimov)
944b21b70ae490a5a746bcc1810a5074d74e9d34 net: don't check if the socket is valid in ConnectSocketDirectly() (Vasil Dimov)
aeac68d036e3cff57ce155f1a904d77f98b357d4 net: don't check if the socket is valid in GetBindAddress() (Vasil Dimov)
5ac1a51ee5a57da59f1ff1986b7d9054484d3c80 i2p: avoid using Sock::Get() for checking for a valid socket (Vasil Dimov)

Pull request description:

  _This is a piece of #21878, chopped off to ease review._

  Peeking at the underlying socket file descriptor of `Sock` and checkig if it is `INVALID_SOCKET` is bad encapsulation and stands in the way of testing/mocking/fuzzing.

  Instead use an empty `unique_ptr` to denote that there is no valid socket where appropriate or outright remove such checks where they are not necessary.

  The default constructor `Sock::Sock()` is unnecessary now after recent changes, thus remove it.

ACKs for top commit:
  ajtowns:
    ACK 7df450836969b81e98322c9a09c08b35d1095a25
  jonatack:
    ACK 7df450836969b81e98322c9a09c08b35d1095a25

Tree-SHA512: 9742aeeeabe8690530bf74caa6ba296787028c52f4a3342afd193b05dbbb1f6645935c33ba0a5230199a09af01c666bd3c7fb16b48692a0d185356ea59a8ddbf

848eec09363d1ba8198376eb9654b1a69e3541aa depends: fix unusable memory_resource in macos qt build (fanquake)

Pull request description:

  See https://codereview.qt-project.org/c/qt/qtbase/+/482392.

  Fixes #28566.

ACKs for top commit:
  hebasto:
    ACK 848eec09363d1ba8198376eb9654b1a69e3541aa.

Tree-SHA512: dd902f7abb09bda3800d78fe58937b4426d974c24ba321b979eba0d6da30fa0c661b4ed629afab827df8f9ab599efc7a288e9f381ec2b3c69d1063d4d4f73f9e

352d5eb2a9e89cff4a2815d94a9d81fcc20c4b2c test: getrawaddrman RPC (0xb10c)
da384a286bd84a97e7ebe7a64654c5be20ab2df1 rpc: getrawaddrman for addrman entries (0xb10c)

Pull request description:

  Inspired by `getaddrmaninfo` (#27511), this adds a hidden/test-only `getrawaddrman` RPC. The RPC returns information on all addresses in the address manager new and tried tables. Addrman table contents can be used in tests and during development.

  The RPC result encodes the `bucket` and `position`, the internal location of addresses in the tables, in the address object's string key. This allows users to choose to consume or to ignore the location information. If the internals of the address manager implementation change, the location encoding might change too.

  ```
  getrawaddrman

  EXPERIMENTAL warning: this call may be changed in future releases.

  Returns information on all address manager entries for the new and tried tables.

  Result:
  {                                  (json object)
    "table" : {                      (json object) buckets with addresses in the address manager table ( new, tried )
      "bucket/position" : {          (json object) the location in the address manager table (<bucket>/<position>)
        "address" : "str",           (string) The address of the node
        "port" : n,                  (numeric) The port number of the node
        "network" : "str",           (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the address
        "services" : n,              (numeric) The services offered by the node
        "time" : xxx,                (numeric) The UNIX epoch time when the node was last seen
        "source" : "str",            (string) The address that relayed the address to us
        "source_network" : "str"     (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the source address
      },
      ...
    },
    ...
  }

  Examples:
  > bitcoin-cli getrawaddrman
  > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawaddrman", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  ```

ACKs for top commit:
  willcl-ark:
    reACK 352d5eb2a9
  amitiuttarwar:
    reACK 352d5eb2a9e
  stratospher:
    reACK 352d5eb.
  achow101:
    ACK 352d5eb2a9e89cff4a2815d94a9d81fcc20c4b2c

Tree-SHA512: cc462666b5c709617c66b0e3e9a17c4c81e9e295f91bdd9572492d1cb6466fc9b6d48ee805ebe82f9f16010798370effe5c8f4db15065b8c7c0d8637675d615e

bdee8589644fac121320e95f53457c3ddfc71e1b typo: in packages.md (Erik McKelvey)

Pull request description:

  Removed extra word `the` in packages.md

ACKs for top commit:
  fanquake:
    ACK bdee8589644fac121320e95f53457c3ddfc71e1b

Tree-SHA512: 14a745e5f8ad97f38c21c7b80e88592f84f50d87bc71930c1212fb9ba5a46b129ffec0aa1dd53780f872c0067b58bd2a65ed9def4d9f5f50dc8c7d8e34a429d4

68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483 http: bugfix: track closed connection (stickies-v)
084d0372311e658a486622f720d2b827d8416591 http: log connection instead of request count (stickies-v)
41f9027813f849a9fd6a1479bbb74b9037990c3c http: refactor: use encapsulated HTTPRequestTracker (stickies-v)

Pull request description:

  #26742 significantly increased the http server shutdown speed, but also introduced a bug (#27722 - see https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1559453982 for steps to reproduce on master) that causes http server shutdown to halt in case of a remote client disconnection. This happens because `evhttp_request_set_on_complete_cb` is never called and thus the request is never removed from `g_requests`.

  This PR fixes that bug, and improves robustness of the code by encapsulating the request tracking logic. Earlier approaches (#27909, #27245, #19434) attempted to resolve this but [imo are fundamentally unsafe](https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1265614783) because of differences in lifetime between an `evhttp_request` and `evhttp_connection`.

  We don't need to keep track of open requests or connections, we just [need to ensure](https://github.com/bitcoin/bitcoin/pull/19420#issue-648067169) that there are no active requests on server shutdown. Because a connection can have multiple requests, and a request can be completed in various ways (the request actually being handled, or the client performing a remote disconnect), keeping a counter per connection seems like the approach with the least overhead to me.

  Fixes #27722

ACKs for top commit:
  vasild:
    ACK 68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483
  theStack:
    ACK 68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483

Tree-SHA512: dfa711ff55ec75ba44d73e9e6fac16b0be25cf3c20868c2145a844a7878ad9fc6998d9ff62d72f3a210bfa411ef03d3757b73d68a7c22926e874c421e51444d6

ce6df7df9bab2405cfe7d6e382f5682cf30de476 bench: Add SHA256 implementation specific benchmarks (Hennadii Stepanov)
5f72417176cfffece9a5aa11e97d5a6599c51e7a Add ability to specify SHA256 implementation for benchmark purposes (Hennadii Stepanov)

Pull request description:

  On the master branch, only the best available `SHA256` implementation is being benchmarked. This PR makes `bench_bitcoin` benchmark all `SHA256` implementations that are available on the system.

  For  example:
  - on Linux:
  ```
  $ ./src/bench/bench_bitcoin -filter=SHA.*
  Using the 'x86_shani(1way,2way)' SHA256 implementation

  |             ns/byte |              byte/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |                1.00 |    1,002,545,462.93 |    0.4% |      0.01 | `SHA1`
  |                2.91 |      344,117,991.18 |    0.1% |      0.03 | `SHA256 using the 'standard' SHA256 implementation`
  |                2.21 |      453,081,794.40 |    0.1% |      0.02 | `SHA256 using the 'sse4(1way),sse41(4way)' SHA256 implementation`
  |                2.21 |      453,396,506.58 |    0.1% |      0.02 | `SHA256 using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
  |                0.53 |    1,870,520,687.49 |    0.1% |      0.01 | `SHA256 using the 'x86_shani(1way,2way)' SHA256 implementation`
  |                7.90 |      126,627,134.33 |    0.0% |      0.01 | `SHA256D64_1024 using the 'standard' SHA256 implementation`
  |                3.94 |      253,850,206.07 |    0.0% |      0.01 | `SHA256D64_1024 using the 'sse4(1way),sse41(4way)' SHA256 implementation`
  |                1.40 |      716,247,553.38 |    0.4% |      0.01 | `SHA256D64_1024 using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
  |                1.26 |      792,706,270.13 |    0.9% |      0.01 | `SHA256D64_1024 using the 'x86_shani(1way,2way)' SHA256 implementation`
  |                6.75 |      148,172,097.64 |    0.2% |      0.01 | `SHA256_32b using the 'standard' SHA256 implementation`
  |                4.90 |      204,156,289.96 |    0.1% |      0.01 | `SHA256_32b using the 'sse4(1way),sse41(4way)' SHA256 implementation`
  |                4.90 |      204,101,274.22 |    0.1% |      0.01 | `SHA256_32b using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
  |                1.70 |      589,052,595.35 |    0.4% |      0.01 | `SHA256_32b using the 'x86_shani(1way,2way)' SHA256 implementation`
  |                2.21 |      453,441,736.14 |    1.0% |      0.02 | `SHA3_256_1M`
  |                1.92 |      521,807,101.48 |    1.0% |      0.02 | `SHA512`
  ```

  - on macOS (M1):
  ```
  % ./src/bench/bench_bitcoin -filter=SHA.\*
  Using the 'arm_shani(1way,2way)' SHA256 implementation

  |             ns/byte |              byte/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |                1.36 |      737,644,274.00 |    0.6% |      0.02 | `SHA1`
  |                3.08 |      324,556,777.15 |    0.2% |      0.03 | `SHA256 using the 'standard' SHA256 implementation`
  |                0.45 |    2,198,104,135.18 |    0.3% |      0.01 | `SHA256 using the 'arm_shani(1way,2way)' SHA256 implementation`
  |                8.84 |      113,131,299.18 |    0.0% |      0.01 | `SHA256D64_1024 using the 'standard' SHA256 implementation`
  |                0.94 |    1,059,406,239.36 |    0.0% |      0.01 | `SHA256D64_1024 using the 'arm_shani(1way,2way)' SHA256 implementation`
  |                6.17 |      162,050,659.51 |    0.2% |      0.01 | `SHA256_32b using the 'standard' SHA256 implementation`
  |                1.15 |      866,637,155.98 |    0.0% |      0.01 | `SHA256_32b using the 'arm_shani(1way,2way)' SHA256 implementation`
  |                1.69 |      592,636,491.59 |    0.2% |      0.02 | `SHA3_256_1M`
  |                1.89 |      528,785,775.66 |    0.0% |      0.02 | `SHA512`
  ```

  Found it useful, while working on https://github.com/bitcoin/bitcoin/pull/24773.

ACKs for top commit:
  martinus:
    ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476. I would have created a helper function in the test to avoid the code duplication for each test, but that's just me nitpicking. Here are results from my Ryzen 7950X, with `./src/bench/bench_bitcoin -filter="SHA256.*" -min-time=1000`:
  MarcoFalke:
    review ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476 🏵
  sipa:
    ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476

Tree-SHA512: e3de50e11b9a3a0d1e05583786041d4dc9afa2022e2115d75d6d1f63b11f62f6336f093001e53a631431d558c4dae29c596755c9e2d6aa78c382270116cc1f7f

aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b ci: Only run functional tests on windows in master (Fabian Jahr)

Pull request description:

  This idea was discussed [here](https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1740841988).

ACKs for top commit:
  hebasto:
    ACK aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b

Tree-SHA512: 89fd6352b585bae3538d5350b0404c216a8225fe356d408c1ebe3394e7b9a190d65639f4eef310056e020909928d7a1f2de25585c97d2ac087d1a9f72af281eb

7e4003226030a04a19c718a4b1b83b4ca40ca33f tests: assumeutxo: accept final height from either chainstate (James O'Beirne)
5bd2010f024b5bcccf1d57bae6fc36c53f5facc5 test: assumeutxo: avoid race in functional test (James O'Beirne)
7005a01c19001ab5821731597656f8bc5e8c11e3 test: add wait_for_connect to BitcoinTestFramework.connect_nodes (James O'Beirne)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/28585.

  Fixes a few races within the assumeutxo tests:
  - In general, `-stopatheight` can't be used with `connect_nodes` safely because the latter performs blocking assertions that are racy with the stopatheight triggering.
  - Now that the snapshot chainstate is listed as `normal` after background validation, accept the final height from either chainstate.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 7e4003226030a04a19c718a4b1b83b4ca40ca33f
  fjahr:
    Code review ACK 7e4003226030a04a19c718a4b1b83b4ca40ca33f
  achow101:
    ACK 7e4003226030a04a19c718a4b1b83b4ca40ca33f
  ryanofsky:
    Code review ACK 7e4003226030a04a19c718a4b1b83b4ca40ca33f

Tree-SHA512: 8cbd2a0ca8643f94baa0ae3561dcf68c3519d5ba851c6049e1768f28cae6434f47ffc28d404bf38ed11030ce3f00aae0a8be3f6d563e6ae6680d83c928a173d8

6988a2f097e9af50e1b4222550b2593bfc5685ea build: Update qt package up to 5.15.10 (Hennadii Stepanov)

Pull request description:

  The Qt 5.15.10 contains at least three important for us fixes:
  - https://github.com/qt/qtbase/commit/8bb90ab760310c9bc82b6ecbe068532fcb4e7822, which allows us to drop the [`dont_hardcode_x86_64.patch`](https://github.com/bitcoin/bitcoin/blob/fd8ab08558ccfb62f2d0a64d2d0d4dcefba977bc/depends/patches/qt/dont_hardcode_x86_64.patch)
  - https://github.com/qt/qtbase/commit/8467beddb7239cc213ae13900fa30e3d26df5e78, which allows us to drop the [`fix_montery_include.patch`](https://github.com/bitcoin/bitcoin/blob/fd8ab08558ccfb62f2d0a64d2d0d4dcefba977bc/depends/patches/qt/fix_montery_include.patch)
  - https://github.com/qt/qtbase/commit/df08a21fa4e7139d46ec68bcf264c922789c4f3a, which addresses https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1743519614

ACKs for top commit:
  fanquake:
    ACK 6988a2f097e9af50e1b4222550b2593bfc5685ea.

Tree-SHA512: 838c44cf0d7508714f35887bb1f0983e59d96764f08f467bbd03d102d7152e797daeedc178afb03141115cc3be623e4ef7c4c4c65c0a8d1f0bb9702c9a6f8fc6

d27b9a2248476439ddab7700327f074005a810d5 test: fix feature_init.py file perturbation (Martin Zumsande)
ad66ca1e475d2546dbbda206465307613108a15d init: abort loading of blockindex in case of missing height. (Martin Zumsande)

Pull request description:

  When the block index database is non-contiguous due to file corruption (i.e. it contains indexes of height `x-1` and `x+1`, but not `x`), bitcoind can currently crash with an assert in `BuildSkip()` / `GetAncestor()` during `BlockManager::LoadBlockIndex()`:
  ```
  bitcoind: chain.cpp:112: const CBlockIndex* CBlockIndex::GetAncestor(int) const: Assertion `pindexWalk->pprev' failed.
  ```
  This PR changes it such that we instead return an `InitError` to the user.

  I stumbled upon this because I noticed that the file perturbation in `feature_init.py`  wasn't working as intended, which is fixed in the second commit:
  * Opening the file twice in one `with` statement would lead to `tf_read` being empty, so the test wouldn't perturb anything but replace the file with a new one. Fixed by first opening for read, then for write.
  * We need to restore the previous state after perturbations, so that only the current perturbation is active and not a mix of the current and previous ones.
  * I also added `checkblocks=200` to the startup parameters so that corruption in earlier blocks of `blk00000.dat` is detected during init verification and not ignored.

  After fixing `feature_init.py` like that I'd run into the `assert` mentioned above (so running the testfix from the second commit without the first one is a way to reproduce it).

ACKs for top commit:
  achow101:
    ACK d27b9a2248476439ddab7700327f074005a810d5
  furszy:
    Code ACK d27b9a224
  fjahr:
    Code review ACK d27b9a2248476439ddab7700327f074005a810d5

Tree-SHA512: 2e54da6030c5813c86bd58f816401e090bb43c5b834764a5e3c0e55dbfe09e423f88042cab823db3742088204b274d4ad2abf58a3832a4b18328b11a30bf7094

58c9b50a952951cb326c99ba86cb706a1e7d533e gui: Add wallet name to address book page (pablomartin4btc)

Pull request description:

  It fixes bitcoin-core/gui#756.

  Each address book page window it's now labeled with the wallet name they were opened with, so the user can easily identify which addresses belong to which wallet even when there are many windows opened. It's a helpful enhancement for users managing multiple wallets.

  ![image](https://github.com/bitcoin-core/gui/assets/110166421/628e37bb-87b9-42fb-9158-bffdd8428bcb)

ACKs for top commit:
  hebasto:
    ACK 58c9b50a952951cb326c99ba86cb706a1e7d533e, tested on Ubuntu 22.04.

Tree-SHA512: 82febc020653560281da144cd35c672c49ca9f48b23d173eb19395e9ab4d045500295a9b5f24c82efdbf6e7ea70c87e733207cb6a31d3f84828b27e3b2df558b

ba2e5bfc67dcffca26af9e231652ada1767cbeb2 net: raise V1_PREFIX_LEN from 12 to 16 (Pieter Wuille)

Pull request description:

  A "version" message in the V1 protocol starts with a fixed 16 bytes:
  * The 4-byte network magic
  * The 12-byte command string: "version" plus 5 0x00 bytes

  The current code detects incoming V1 connections by just looking at the first 12 bytes (matching an [earlier version](https://github.com/bitcoin/bips/pull/1496) of BIP324), but 16 bytes is more precise. This isn't an observable difference right now, as a 12 byte prefix ought to be negligible already, but it may become observable with future extensions to the protocol, so make the code match the specification.

ACKs for top commit:
  achow101:
    ACK ba2e5bfc67dcffca26af9e231652ada1767cbeb2
  theStack:
    re-ACK ba2e5bfc67dcffca26af9e231652ada1767cbeb2
  mzumsande:
    Code review ACK ba2e5bfc67dcffca26af9e231652ada1767cbeb2

Tree-SHA512: 64876b03613bd1c5dda82f4ca1b367014365f9ae4cfa30f45c5758a563c68cbea81a98d02ba616c264674c204517aac8b7de94da10f32e77b56267a43710c651

4cafe9f176e93ebb6c38abb12140e8d8be005cbf [test] Make PeerManager's rng deterministic in tests (dergoegge)
fecec3e1c661ba273470ecc5ef12d4c070b53050 [net processing] FeeFilterRounder doesn't own a FastRandomContext (dergoegge)
47520ed209d9341702a0fb6006bee6f63f7da42e [net processing] Make fee filter rounder non-global (dergoegge)
77506f4ac6b3a3d7396a3a6101345019e05b3b10 [net processing] Addr shuffle uses PeerManager's rng (dergoegge)
a648dd79e5ebfdb627d0221b1207862efb664dfc [net processing] PushAddress uses PeerManager's rng (dergoegge)
87c706713e5d1c78bad943a42bf7c69047d28ea5 [net processing] PeerManager holds a FastRandomContext (dergoegge)

Pull request description:

  This lets us avoid some non-determinism in tests (also see #28537).

ACKs for top commit:
  MarcoFalke:
    re-ACK 4cafe9f176e93ebb6c38abb12140e8d8be005cbf  🕗
  glozow:
    concept && light code review ACK 4cafe9f176e93ebb6c38abb12140e8d8be005cbf

Tree-SHA512: 3c18700773d0bc547ccb6442c41567e6f26b0b50fab5b79620da417ec91b9c0ae1395d15258da3aa4a91447b8ce560145dd135e39fbbd0610749e528e665b111

c1e6c542af6d89a499e2a65465865aec651c4d67 descriptors: disallow hybrid public keys (Pieter Wuille)

Pull request description:

  Fixes #28511

  The descriptor documentation (`doc/descriptors.md`) and [BIP380](https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki) explicitly require that hex-encoded public keys start with 02 or 03 (compressed) or 04 (uncompressed). However, the current parsing/inference code permit 06 and 07 (hybrid) encoding as well. Fix this.

ACKs for top commit:
  darosior:
    ACK c1e6c542af6d89a499e2a65465865aec651c4d67
  achow101:
    ACK c1e6c542af6d89a499e2a65465865aec651c4d67

Tree-SHA512: 23b674fb420619b2536d12da10008bb87cf7bc0333ec59e618c0d02c3574b468cc71248475ece37f76658d743ef51e68566948e903bca79fda5f7d75416fea4d

a9ef702a877a964bac724a56e2c0b5bee4ea7586 assumeutxo: change getchainstates RPC to return a list of chainstates (Ryan Ofsky)

Pull request description:

  Current `getchainstates` RPC returns "normal" and "snapshot" fields which are not ideal because it requires new "normal" and "snapshot" terms to be defined, and the definitions are not really consistent with internal code. (In the RPC interface, the "snapshot" chainstate becomes the "normal" chainstate after it is validated, while in internal code there is no "normal chainstate" and the "snapshot chainstate" is still called that temporarily after it is validated).

  The current `getchainstates` RPC is also awkward to use if you to want information about the most-work chainstate, because you have to look at the "snapshot" field if it exists, and otherwise fall back to the "normal" field.

  Fix these issues by having `getchainstates` just return a flat list of chainstates ordered by work, and adding a new chainstate "validated" field alongside the existing "snapshot_blockhash" field so it is explicit if a chainstate was originally loaded from a snapshot, and whether the snapshot has been validated.

  This change was motivated by comment thread in https://github.com/bitcoin/bitcoin/pull/28562#discussion_r1344154808

ACKs for top commit:
  Sjors:
    re-ACK a9ef702a877a964bac724a56e2c0b5bee4ea7586
  jamesob:
    re-ACK a9ef702
  achow101:
    ACK a9ef702a877a964bac724a56e2c0b5bee4ea7586

Tree-SHA512: b364e2e96675fb7beaaee60c4dff4b69e6bc2d8a30dea1ba094265633d1cddf9dbf1c5ce20c07d6e23222cf1e92a195acf6227e4901f3962e81a1e53a43490aa

f0cebbdb2a1a3c2f0facd88963484ad6fd5851db qt: enable -ltcg for windows HOST (fanquake)

Pull request description:

  Patch around multiple definition issues in Qt, and enable `-ltcg` when using `LTO=1`.

  Split from #25391.

ACKs for top commit:
  hebasto:
    ACK f0cebbdb2a1a3c2f0facd88963484ad6fd5851db

Tree-SHA512: 2d6e34779f360bf6dfea4f70fc9004a16e95da79716fcb3046afbf2b01317b7e16965cb51b967b7b5fb64549306c5f48cf59082884289c52016bc1e86949e062

05af4dfa50c229c8533d9a71e046c9387e1cdb27 test: Use feerate higher than minrelay fee in wallet_fundraw (Andrew Chow)

Pull request description:

  The external input weight test in wallet_fundrawtransaction.py made transactions at the minimum relay fee. However due to ECDSA sometimes making a shorter signature than expected, the size estimate (and therefore the funded fee) ends up being a little bit too low, which results in the final transaction being under the min relay fee. We can compensate for this by just using a feerate higher than the minrelayfee as the actual feerate itself does not matter in this test.

  Fixes #28437

ACKs for top commit:
  glozow:
    utACK 05af4dfa50c229c8533d9a71e046c9387e1cdb27, seems right to me

Tree-SHA512: 3e08f052db32d891515d32b27b2d7c5bcbfc77d5ea457eefc75e8aa6d40960278e537c1e8bffe7ddc211949c78e14145a671a8d34e7e1bb15438773cb0d9e89d

5f504065544133a47da5a7a240675c23eceb0799 Adjust Gradle properties (Hennadii Stepanov)

Pull request description:

  On the master branch @ d2b8c5e1234cdaff84bd1f60aea598d219cdac5e, building the `apk` target fails:
  ```
  $ make -C src/qt apk
  ...
  > Task :compileDebugJavaWithJavac FAILED
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:690: error: cannot find symbol
                  Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
                                                                                ^
    symbol:   variable R
    location: class VERSION_CODES
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:692: error: cannot find symbol
                          : m_activity.getDisplay();
                                      ^
    symbol:   method getDisplay()
    location: variable m_activity of type Activity
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:833: error: cannot find symbol
          float refreshRate = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
                                                                          ^
    symbol:   variable R
    location: class VERSION_CODES
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtActivityDelegate.java:835: error: cannot find symbol
                  : m_activity.getDisplay().getRefreshRate();
                              ^
    symbol:   method getDisplay()
    location: variable m_activity of type Activity
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtLayout.java:95: error: cannot find symbol
          Display display = (Build.VERSION.SDK_INT < Build.VERSION_CODES.R)
                                                                        ^
    symbol:   variable R
    location: class VERSION_CODES
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/QtLayout.java:97: error: cannot find symbol
                  : ((Activity)getContext()).getDisplay();
                                            ^
    symbol:   method getDisplay()
    location: class Activity
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/ExtractStyle.java:418: error: cannot find symbol
              if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q)
                                                             ^
    symbol:   variable Q
    location: class VERSION_CODES
  /home/hebasto/git/gui/src/qt/android/src/org/qtproject/qt5/android/ExtractStyle.java:421: error: cannot find symbol
                  numStates = stateList.getStateCount();
                                       ^
    symbol:   method getStateCount()
    location: variable stateList of type StateListDrawable
  Note: Some input files use or override a deprecated API.
  Note: Recompile with -Xlint:deprecation for details.
  8 errors

  FAILURE: Build failed with an exception.

  * What went wrong:
  Execution failed for task ':compileDebugJavaWithJavac'.
  > Compilation failed; see the compiler error output for details.

  * Try:
  Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

  * Get more help at https://help.gradle.org

  Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
  Use '--warning-mode all' to show the individual deprecation warnings.
  See https://docs.gradle.org/6.6.1/userguide/command_line_interface.html#sec:command_line_warnings

  BUILD FAILED in 827ms
  ...
  ```

  Fixing it by updating the Gradle tool's properties.

ACKs for top commit:
  fanquake:
    ACK 5f504065544133a47da5a7a240675c23eceb0799 - seems fine.

Tree-SHA512: 52e59fe1c69841370ce2eb670f3618182bf2843582074af4895b8ecb6e5f70dc3fe4eecbffa212efaa534b423ced5b75020f6f09917b52f452121c1e55fbcaac

fac88a874f57bfbedbaffaf43a01b3a74be8d875 ci: Avoid cache depends/build (MarcoFalke)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/28365 (maybe?)

ACKs for top commit:
  hebasto:
    ACK fac88a874f57bfbedbaffaf43a01b3a74be8d875

Tree-SHA512: c121f335f4b7d1f5f8f0508a17cee4d33dcf43c3928f9003a01dcc9ed0c45d1d912f7ff976e605b06c2f293ed21371d2dcace5562fe466bc444144790353bdd3

88c8e3a0e4d6bee015a348536c6e12a2c7835896 github actions: Fix test-one-commit when parent of head is merge commit (Ryan Ofsky)

Pull request description:

  Instead of figuring out the commit *after* the last merge and rebasing on that with a ~1 suffix, just figure out the last merge commit directly and rebase on it. This way, if HEAD happens to be a merge commit, the rebase just succeeds immediately without blank variables or errors.

  Explanation of the problem from https://github.com/bitcoin/bitcoin/pull/28497#issuecomment-1743430631:

  > The problem is that the PR only contains a one commit after the last merge, so the job _should_ be skipped, but the `pull_request.commits != 1` check is not smart enough to skip it because the PR is based on another PR and has merge ancestor commits. So specifically what happens is that after HEAD~ is checked out, the new HEAD  is a merge commit, so the range `$(git log --merges -1 --format=%H)..HEAD` is equivalent to HEAD..HEAD, which is empty, so the `COMMIT_AFTER_LAST_MERGE` variable is empty and the rebase command fails.

  Note: In the current version of this PR, the "test each commit" job is skipped, because this PR only contains a single commit. But I manually verified the code works in earlier versions of the PR that included dummy commits.

ACKs for top commit:
  maflcko:
    lgtmrecr ACK 88c8e3a0e4d6bee015a348536c6e12a2c7835896
  RandyMcMillan:
    utACK 88c8e3a

Tree-SHA512: a6865b5c8b96eb0b622b3255971a3cf050dd0f5a356cdfcf7f0cbb659e4a363612e8e62b3ae4fd6b5d9a40bc29176891bc4690659b026c5ef8feea25c8e263cc

ed52e71176fc97c6ed01e3eebd85acdec54b4448 Periodically check disk space to avoid corruption (Aurèle Oulès)
7fe537f7a48675b1d25542bee6f390d665547580 Implement CCoinsViewErrorCatcher::HaveCoin (Aurèle Oulès)

Pull request description:

  Attempt to fix #26112.

  As suggested by sipa in https://github.com/bitcoin/bitcoin/issues/26112#issuecomment-1249683401:
  > CCoinsViewErrorCatcher, the wrapper class used around CCoinsViewDB that's supposed to detect these problems and forcefully exit the application, has an override for GetCoins. But in CheckTxInputs, HaveInputs is first invoked, which on its turn calls HaveCoin. HaveCoin is implemented in CCoinsViewDB, but not in CCoinsViewErrorCatcher, and thus the disk read exception escapes.
  > A solution may be to just add an override for HaveCoin in CCoinsViewErrorCatcher.

  I implemented `CCoinsViewErrorCatcher::HaveCoin` and also added a periodic disk space check that shutdowns the node if there is not enough space left on disk, the minimum here is 50MB.

  For reviewers, it's possible to saturate disk space to test the PR by creating large files with `fallocate -l 50G test.bin`

ACKs for top commit:
  achow101:
    ACK ed52e71176fc97c6ed01e3eebd85acdec54b4448
  w0xlt:
    Code Review ACK https://github.com/bitcoin/bitcoin/pull/26331/commits/ed52e71176fc97c6ed01e3eebd85acdec54b4448
  sipa:
    utACK ed52e71176fc97c6ed01e3eebd85acdec54b4448

Tree-SHA512: 456aa7b996023df42b4fbb5158ee429d9abf7374b7b1ec129b21aea1188ad19be8da4ae8e0edd90b85b7a3042b8e44e17d3742e33808a4234d5ddbe9bcef1b78

57131bfa3cfd9e5826b5c557aba8aa03bf41f833 docs: fix typo (vuittont60)

Pull request description:

ACKs for top commit:
  maflcko:
    lgtm ACK 57131bfa3cfd9e5826b5c557aba8aa03bf41f833
  hebasto:
    ACK 57131bfa3cfd9e5826b5c557aba8aa03bf41f833, the `codespell` is powerless to catch this typo.
  jarolrod:
    ACK 57131bfa3cfd9e5826b5c557aba8aa03bf41f833

Tree-SHA512: 816dfc5ff64531ea92acf35feca2286a71c75344df2524ff003a3d375e60100b8531e1678be0ed11863d03ab522d5733d8a0bf4b6f5f79c495a65246fe0b697f

78d3062b68988f5094c61a845be756788933c752 ci: Install Android API 31 platform as Qt expects (Hennadii Stepanov)

Pull request description:

  When building the `qt` package, it expects that the default (in Qt's view) Android API platform is installed.

  During the recent Qt version [update](https://github.com/bitcoin/bitcoin/pull/28561), it has been changed:
  ```diff
  --- a/mkspecs/features/android/sdk.prf
  +++ b/mkspecs/features/android/sdk.prf
  @@ -1,6 +1,6 @@
   API_VERSION_TO_USE = $$(ANDROID_API_VERSION)
   isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = $$API_VERSION
  -isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = android-28
  +isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = android-31

   ANDROID_JAR_FILE = $$ANDROID_SDK_ROOT/platforms/$$API_VERSION_TO_USE/android.jar
   !exists($$ANDROID_JAR_FILE) {
  ```

  This PR fixes the CI for the Android task and addresses https://github.com/bitcoin/bitcoin/pull/28561#issuecomment-1749180177.

  Qt [docs](https://doc.qt.io/qt-5/android.html) still claim that Android API Level 21 and up are supported, however, I did not test every possible configuration.

  NOTE: https://github.com/bitcoin/bitcoin/pull/28611 is still valid.

ACKs for top commit:
  maflcko:
    lgtm ACK 78d3062b68988f5094c61a845be756788933c752
  jarolrod:
    tACK 78d3062b68988f5094c61a845be756788933c752

Tree-SHA512: 781fba6d80aae7e6500854de14af0d30169c258e395b9e482a5430a7b4a2211a6181f8c9ee58543c896b431abf09e3e7c5573b9672ed128658f11f98a2006e7e

3bb51c29df596aab2c1fde184667cee435597715 test: BIP324: add check for missing garbage terminator detection (Sebastian Falbesoner)

Pull request description:

  This PR adds test coverage for the "missing garbage terminator" detection on incoming v2 transport (BIP324) connections:
  https://github.com/bitcoin/bitcoin/blob/04265ba9378efbd4c35b33390b1e5cf246d420a9/src/net.cpp#L1205-L1209

  Note that this always happens at the same exact amount of bytes sent in (after 64 + 4095 + 16 = 4175 bytes), if at no point, the last 16 bytes of potential authentication data match the garbage, i.e. all the previous bytes after the ellswift pubkey. To keep it simple, we just send in zero-value bytes here and verify that the detection hits exactly after the last bytes is sent.

  AFAICT, with this PR all the v2 transport errors that can be triggered in this simple way of "just open a socket and send in a fixed byte-string" are covered. For more advanced test, we need BIP324 cryptography in the test framework in order to perform a v2 handshake etc. (PRs #28374, #24748).

ACKs for top commit:
  sipa:
    utACK 3bb51c29df596aab2c1fde184667cee435597715
  laanwj:
    ACK 3bb51c29df596aab2c1fde184667cee435597715

Tree-SHA512: f88275061c7c377a3d9f2608452671afc26deb6d5bd5be596de987c7e5042555153ffe681760c33bce2b921ae04e50f349ea0128a677e6443a95a079e52cdc5f

850670e3d63ed7d04b417a43cb8ab06292aa2c23 test: don't run old binaries under valgrind (Sjors Provoost)

Pull request description:

  Some, but not all, backward compatibility tests fail for me and it seems useless to run old release binaries under valgrind anyway.

  Can be tested by running `test/functional/feature_txindex_compatibility.py --valgrind --timeout-factor=10` with and without this PR.

  —
  The previous version of this PR disabled these test entirely under valgrind. The current version does run the test, but starts the old binaries without valgrind.

ACKs for top commit:
  maflcko:
    lgtm ACK 850670e3d63ed7d04b417a43cb8ab06292aa2c23

Tree-SHA512: ebdf461083f1292528e6619963b910f486b60b4f6b183f0aea2c8bfcafa98caeb204d138700cd288450643bcec5e49e12b89f2f7537fccdf495a2a33acd9cea0

4077e43bf62e5afe90d204b9ede9290ef54dee0f test: fix usdt undeclared function errors on mantis (willcl-ark)

Pull request description:

  This is one way to fix #28600

  Recently usage of undeclared functions became an error rather than a warning, in C2x. https://reviews.llvm.org/D122983?id=420290

  This change has migrated into the build tools of Ubuntu 23.10 which now causes the USDT tests to fail to compile, see
  https://github.com/bitcoin/bitcoin/issues/28600

  I think there are various potential fixes:

  1. Manually declare the functions we use
  2. Fix imports so that manual declarations aren't needed
  3. Revert the new C2X behaviour and don't error on implicit function declarations

  I would have preferred solution 2, but I believe this will require changes to the upstream bcc package. Having played with the imports I can get things working in a standalone C program, using system headers, but when building the program from a python context as we do in the test it uses its own headers (bundled with the python lib) rather than the system ones, and manually importing (some) system headers results in definition mismatches. I also investigated explicitly importing required headers from the package, which use paths like `#import </virtual/bcc/bcc_helpers.h>`, but this seems more obtuse and brittle than simply ignoring the warning.

  Therefore I think that until the upstream python pacakge fixes their declarations, we should fix this by setting `-Wno-error=implicit-function-declaration` for the tracing programs.

  cc maflcko 0xB10C

ACKs for top commit:
  maflcko:
    lgtm ACK 4077e43bf62e5afe90d204b9ede9290ef54dee0f

Tree-SHA512: 8368bb1155e920a95db128dc893267f8dab64f1ae53f6d63c6d9294e2e4e92bef8515e3697e9113228bedc51c0afdbc5bbcf558c119bf0eb3293dc2ced86b435

79789ccafe101d3bb05fffe08610d2103e3f3060 ci: Drop no longer needed `NOLINTNEXTLINE` (Hennadii Stepanov)

Pull request description:

  After recent tool updates in the "tidy" CI task, the one instance of `NOLINTNEXTLINE` is not required anymore.

ACKs for top commit:
  fanquake:
    ACK 79789ccafe101d3bb05fffe08610d2103e3f3060

Tree-SHA512: 382f3e0570f36a481498ac118ee30a34aabfd6285b21a67c19c74b240305315c5a118fb70f172b843a086262433462d45998e78306697be0ddfb20094733ee98

faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 test: Fuzz merge with -use_value_profile=0 for now (MarcoFalke)

Pull request description:

  Seems odd that this has to be done, but for now there are (unknown) size limits on the qa-assets repo. Also, a larger size means that cloning and iterating over the files takes a longer time.

  Not sure how to measure the net impact of this, but with some backups reverting this commit, it can be limited on the downside?

ACKs for top commit:
  dergoegge:
    ACK faa190b1efbdfdb9b12a7bfa7f732b5471a02e64

Tree-SHA512: 9f8b3f4526f60e4ff6fca97859a725d145a8339c216bd15c92fad7e53f84308745fee47727527de459c0245ef9d474a9dc836fee599ab2b556b519bd900b9a33

fa2c894cbb41a64371717139fb3c3ddfb9bb8b19 ci: move-only CI_CONTAINER_ID to 02_run_container.sh (MarcoFalke)
fa695b4df069425414fd26b2ddc08d72a6b506f6 ci: Work around podman stop bug (MarcoFalke)
fa09a031c1eb8abcb9a04cacdf5629f95ffc77f8 ci: Add set -ex to 02_run_container.sh (MarcoFalke)
fac9abbf475a1de6f9f39ddede9a6a59bbd1cff4 ci: Rename 04_install to 02_run_container (MarcoFalke)

Pull request description:

  Sometimes, it seems that `podman stop` does not work. Presumably, it falls back to `podman kill`, which is async.

  Try to work around this intermittent issue by using the `rm --force` over `stop`.

  Example failing log https://cirrus-ci.com/task/4549784611061760?logs=ci#L238:

  ```
  Restart docker before run to stop and clear all containers started with --rm
  ++ podman container stop --all
  e4eca0766f87864d89fc230aa884a238c214cfbcd44cf76a4dbdb2a30c982009
  ++ echo 'Prune all dangling images'
  Prune all dangling images
  ++ docker image prune --force
  Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
  +++ docker run --cap-add LINUX_IMMUTABLE --rm --interactive --detach --tty --mount type=bind,src=/tmp/cirrus-build-1970593815,dst=/tmp/cirrus-build-1970593815,readonly --mount type=volume,src=ci_macos_cross_ccache,dst=/tmp/ccache_dir --mount type=volume,src=ci_macos_cross_depends,dst=/ci_container_base/depends --mount type=volume,src=ci_macos_cross_previous_releases,dst=/ci_container_base/prev_releases --env-file /tmp/env --name ci_macos_cross ci_macos_cross
  Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
  time="2023-09-27T20:55:39Z" level=warning msg="The input device is not a TTY. The --tty and --interactive flags might not work properly"
  Error: creating container storage: the container name "ci_macos_cross" is already in use by e4eca0766f87864d89fc230aa884a238c214cfbcd44cf76a4dbdb2a30c982009. You have to remove that container to be able to reuse that name: that name is already in use

ACKs for top commit:
  hebasto:
    ACK fa2c894cbb41a64371717139fb3c3ddfb9bb8b19, I have reviewed the code and tested it locally.

Tree-SHA512: 31fca340c6bedaadf4dd51fa745d9b3969042cebc0c7c904ef18af3f2f986039ec4354ccdff1422fbf77cf223e4423857368dce53cfa67ef15c76b78d007eace

61a6c3b0e9a8dab5c5f845af4becde817539133c build: add `-mbranch-protection=bti` to aarch64 hardening flags (fanquake)

Pull request description:

  This is a simpler (less hardening) version of https://github.com/bitcoin/bitcoin/pull/24123.

  You can inspect binaries using `readelf -n`, and look for BTI in a `.note.gnu.property`. i.e
  ```bash
  readelf -n src/bitcoin-cli

  Displaying notes found in: .note.gnu.property
    Owner                Data size Description
    GNU                  0x00000010NT_GNU_PROPERTY_TYPE_0
        Properties: AArch64 feature: BTI
  ```

  Related to https://github.com/bitcoin/bitcoin/issues/19075.

ACKs for top commit:
  TheCharlatan:
    utACK 61a6c3b0e9a8dab5c5f845af4becde817539133c

Tree-SHA512: 64504de44e91d853165daf4111dca905d8eb9ef3f4bfb0d447c677b02c9100dbd56f13e6fe6539fb06c2343a094229591ac5d1bd9e184b32b512c0ac3f9bac36

fafde92f84fb7c245bc3c1cd946a32c891861e5e test: Check snapshot file with wrong number of coins (MarcoFalke)
faa90f6e7b6b8c531e1ae142a5c2f568b48502a9 refactor: Remove unused nchaintx from SnapshotMetadata constructor (MarcoFalke)

Pull request description:

  See commit messages

ACKs for top commit:
  Sjors:
    utACK fafde92f84fb7c245bc3c1cd946a32c891861e5e
  theStack:
    ACK fafde92f84fb7c245bc3c1cd946a32c891861e5e

Tree-SHA512: 9ed2720b50d1c0938f30543ba143e1a4c6af3a0ff166f8b3eb452e1d99ddee6e3443a4c99f77efe94b8c3eb2feff984bf5259807ee8085e1e0e1e0d1de98227e

36a3004a41aea58f50f3348c5de4eb5a23268788 devtools: test_utxo_snapshots.sh sleep cleanup and documentation (Fabian Jahr)

Pull request description:

  There were bare sleep statements in the script where it was unclear why they were needed and I think they could also be replaced by existing helpers.

ACKs for top commit:
  Sjors:
    utACK 36a3004a41aea58f50f3348c5de4eb5a23268788
  pablomartin4btc:
    utACK https://github.com/bitcoin/bitcoin/commit/36a3004a41aea58f50f3348c5de4eb5a23268788

Tree-SHA512: b6e2fc69cd7babcfa7f413f11304f4d648d6d64b3b526862664ccedb0016ad182b9e886aa4e8c33315e4c18824512e11a4fd6365f9c3c95093967d3ef7687e62

ac4caf3366a85617641394a97aa9f029550d77d4 test: fix `assert_debug_log` call-site bugs, add type checks (Sebastian Falbesoner)

Pull request description:

  Two recently added tests (PR #28625 / commit 2e31250027ac580a7a72221fe2ff505b30836175 and PR #28634 / commit 3bb51c29df596aab2c1fde184667cee435597715) introduced bugs by wrongly using the `assert_debug_log` helper:

  https://github.com/bitcoin/bitcoin/blob/5ea4fc05edde66c5c90383bc054590dfbdb2b645/test/functional/feature_assumeutxo.py#L84-L85 (already fixed in https://github.com/bitcoin/bitcoin/pull/28639)

  https://github.com/bitcoin/bitcoin/blob/5ea4fc05edde66c5c90383bc054590dfbdb2b645/test/functional/p2p_v2_transport.py#L148
  https://github.com/bitcoin/bitcoin/blob/5ea4fc05edde66c5c90383bc054590dfbdb2b645/test/functional/p2p_v2_transport.py#L159

  Instead of passing the expected debug string in a list as expected, it was passed as bare string, which is then interpretered as a list of characters, very likely leading the debug log assertion pass even if the intended message is not appearing. Thanks to maflcko for discovering: https://github.com/bitcoin/bitcoin/pull/28625#discussion_r1356489861

  In order to avoid bugs like this in the future, enforce that the `{un}expected_msgs` parameters are lists, as discussed in https://github.com/bitcoin/bitcoin/pull/28625#discussion_r1356864233. Using mypy might be an alternative, but I guess it takes quite a bit of effort to properly integrate this into CI for the whole functional test suite (including taking care of false-positives), so I decided to go with the simpler "manual asserts" hack. Suggestions are very welcome of course.

ACKs for top commit:
  achow101:
    ACK ac4caf3366a85617641394a97aa9f029550d77d4
  maflcko:
    lgtm ACK ac4caf3366a85617641394a97aa9f029550d77d4
  dergoegge:
    ACK ac4caf3366a85617641394a97aa9f029550d77d4

Tree-SHA512: a9677af76a0c370e71f0411339807b1dc6b2a81763db4ec049cd6d766404b916e2bdd002883db5a79c9c388d7d8ebfcbd5f31d43d50be868eeb928e3c906a746

e44c574650827f18e12ac0ba378c0a19d23a07b4 ci: always prune all dangling bitcoin-ci-test images (stickies-v)
ce1699706e98201db73209ee495051359b1d0703 ci: add label to docker images (stickies-v)

Pull request description:

  Follow-up from https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210209382.

  Labeling the docker images produced by the CI allows us/the user to apply batch operations to all images (including dangling ones) produced by the ci without affecting other, non-bitcoin-ci images. With labeling, we can safely always prune dangling bitcoin-ci-test images without checking for `RESTART_CI_DOCKER_BEFORE_RUN`, which we enable on our persistent runners.

ACKs for top commit:
  fanquake:
    utACK e44c574650827f18e12ac0ba378c0a19d23a07b4

Tree-SHA512: 1009fb1be78fbc80b5341ba92eac2991e77d050e1ab6048d1d9a65af73413a6be7afc1e1c764eb3f347f363af31245b93fdb38f6ac016d775aad4a0f36e4c98f

fa858d63a0a5d794aab38c26f60c593513fe08de fuzz: Merge with -set_cover_merge=1 (MarcoFalke)

Pull request description:

  This should be less controversial than commit 151a2b189c3561dda2bb7de809306c1cfeb40e23. The overall size of the qa-assets repo is reduced further from 1.9GB to 1.6GB. Also, the runtime to iterate on the resulting folder is reduced further from ~1699s to ~1149s (N=1).

ACKs for top commit:
  murchandamus:
    crACK fa858d63a0a5d794aab38c26f60c593513fe08de
  dergoegge:
    ACK fa858d63a0a5d794aab38c26f60c593513fe08de

Tree-SHA512: e23fa93bd48f01d11c551b035004c678bd6d76bc24ac7d0d0a7883060804e6711763cbd0cd0ded3aad3e4c40da764decae81c2703388cc11961def3c89a4f9ba

092daa2f9524e371ee6b505519d9b722a2c6361a contrib: add test for macOS linker version to symbol-check (fanquake)
cefbf0bc20dfe932240877b3096300c8c241ad61 depends: update LD64_VERSION to 711 (fanquake)

Pull request description:

  I forgot to do this in https://github.com/bitcoin/bitcoin/commit/7d5815293ed8a3dea68b61a78944e410f02b147f.
  Add a test so it's impossible to forget.

ACKs for top commit:
  TheCharlatan:
    utACK 092daa2f9524e371ee6b505519d9b722a2c6361a
  achow101:
    ACK 092daa2f9524e371ee6b505519d9b722a2c6361a
  jarolrod:
    ACK 092daa2f9524e371ee6b505519d9b722a2c6361a
  hebasto:
    ACK 092daa2f9524e371ee6b505519d9b722a2c6361a.
  laanwj:
    ACK 092daa2f9524e371ee6b505519d9b722a2c6361a

Tree-SHA512: 37f0bdfd6607a7760eabe5efe279532ba0c59c0915161e08d5e3b9a0b7705839d62537d6e17406062f6a0a1db5407575da7cd671e9cb916e422e77f5649c6e2b

348e79f7c6030c9bcbec6bd25e246a779081152a lint: Include test_utxo_snapshots in lint_shell (Fabian Jahr)

Pull request description:

  jamesob excluded `test_utxo_snapshots.sh` from the shell linter with this explanation: "Add the script to the shellcheck exception list since the quoted variables rule needs to be violated in order to get bitcoind to pick up on $EARLY_IBD_FLAGS." However, macrofake pointed out that single lines can be excluded from linting.

  This fixes one fixable rule violation, excludes the rest of the offending lines from the linter and then removes the exclusion of the `test_utxo_snapshots.sh` file. Also adds documentation.

ACKs for top commit:
  Empact:
    ACK 348e79f7c6030c9bcbec6bd25e246a779081152a
  maflcko:
    lgtm ACK 348e79f7c6030c9bcbec6bd25e246a779081152a
  pablomartin4btc:
    tACK 348e79f7c6030c9bcbec6bd25e246a779081152a

Tree-SHA512: a904cc1cc3c94488dfbd39ea69a3ef17259f991708a797009001669448fef81eed086ecbce1ec433988d88baef293849698e2e0eb86a969b949cc7ef93af7b4b

faa5e061c2f63b868367015561fd376b6ef6052c fuzz: Allow multiple --m_dir args (MarcoFalke)

Pull request description:

  This allows to merge the result from several servers (or just several folders) at the same time, instead of having to iterate over them.

  This should also allow the fuzz engine (libFuzzer) to optimize the final merge result more, because all fuzz inputs from all folders are available at the same time.

ACKs for top commit:
  dergoegge:
    tACK faa5e061c2f63b868367015561fd376b6ef6052c

Tree-SHA512: bf0da418b1f7b8a8af16bb7cc1e148b1ccd0f17062ce70758d1ca5b35c3eee77c0c30377d376befdd55480adfd1f1a1073cfc47118e7a710e6760e020abe24bb

fa685715663117955e9bb795cbf79ddbd3dfed19 test: Add assumeutxo test for wrong hash (MarcoFalke)

Pull request description:

  Also:
  * Update test TODOs
  * Fix off-by-4 typo in test, remove `struct` import

ACKs for top commit:
  fjahr:
    utACK fa685715663117955e9bb795cbf79ddbd3dfed19
  theStack:
    Code-review re-ACK fa685715663117955e9bb795cbf79ddbd3dfed19
  pablomartin4btc:
    re ACK fa685715663117955e9bb795cbf79ddbd3dfed19
  ryanofsky:
    Code review ACK fa685715663117955e9bb795cbf79ddbd3dfed19

Tree-SHA512: 877653010efe4e20018827e8ec2801d036e1344457401f0c9e5d55907b817724201dd2e3f0f29505bbff619882c0c2cd731ecdcd209258bcefe11b86ff0205dd

8a553c94098c96cb3679468c2b460be145a0eabf wallet: Add TxStateString function for debugging and logging (Ryan Ofsky)

Pull request description:

  I found this useful while debugging silent conflict between #10102 and #27469 recently

ACKs for top commit:
  ishaanam:
    utACK 8a553c94098c96cb3679468c2b460be145a0eabf
  achow101:
    ACK 8a553c94098c96cb3679468c2b460be145a0eabf
  furszy:
    Code ACK 8a553c9

Tree-SHA512: 87965c66bcb59a21e7639878bb567e583a0e624735721ff7ad1104eed6bb9fba60607d0e3de7be3304232b3a55f48bab7039ea9c26b0e81963e59f9acd94f666

00a52e63946d5a90cdfb68204373d9c23d885161 gui: fix coin control input size accounting for taproot spends (Sebastian Falbesoner)

Pull request description:

  If manual coin control is used in the GUI, the input size accounting for P2TR is currently overshooting, as it still assumes P2WPKH (segwitv0) spends which have a larger witness, as ECDSA signatures are longer and the pubkey also has to be provided. Fix that by adding sizes depending on the witness version. Note that the total accounting including outputs is still off and there is some weird logic involved depending on whether SFFO is used, but it's (hopefully) a first step into the right direction.

ACKs for top commit:
  maflcko:
    lgtm ACK 00a52e63946d5a90cdfb68204373d9c23d885161
  furszy:
    utACK 00a52e6394

Tree-SHA512: 9633642f8473247cc3d8e6e0ef502fd515e1dde0e2939d28d6754d0cececedd6a328df22a3d4c85eb2846fd0417cf224b92594613f6e84ada82d2d7d84fc455f

b22810887b3840ad0fcb424ea7e16d2d195767d9 miniscript: make GetWitnessSize accurate for tapscript (Pieter Wuille)
8be98514080ab816fcb2498ea4bc6f211a2b05e0 test: add tests for miniscript GetWitnessSize (Pieter Wuille)
7ed2b2d430e4dc0d3ba62a30f814df2c7c0c0651 test: remove mutable global contexts in miniscript fuzzer/test (Pieter Wuille)

Pull request description:

  So far, the same algorithm is used to compute an (upper bound on) the maximum witness size for both P2WSH and P2TR miniscript. That's unfortunate, because it means fee estimations for P2TR miniscript will miss out on the generic savings brought by P2TR witnesses (smaller signatures and public keys, specifically).

  Fix this by making the algorithm use script context specification calculations, and add tests for it. Also included is a cleanup for the tests to avoid mutable globals, as I found it hard to reason about what exactly was being tested.

ACKs for top commit:
  achow101:
    ACK b22810887b3840ad0fcb424ea7e16d2d195767d9
  darosior:
    ACK b22810887b3840ad0fcb424ea7e16d2d195767d9

Tree-SHA512: e4bda7376628f3e91cfc74917cefc554ca16eb5f2a0e1adddc33eb8717c4aaa071e56a40f85a2041ae74ec445a7bd0129bba48994c203e0e6e4d25af65954d9e

1b672eb7665cc032f2c285b1cad331dc92685265 doc: add release note for #27460 (new `importmempool` RPC) (Sebastian Falbesoner)

Pull request description:

  This PR adds a missing release note for #27460.

ACKs for top commit:
  glozow:
    ACK 1b672eb7665cc032f2c285b1cad331dc92685265

Tree-SHA512: 89deadbfd6779e6eb19801c9fe7459a9876b920d44e09df102774c1eb8b3c0716462613dc99d1711eda4bd959ea61595b33f4528424ac02cf1af6cb4e5f1f0e9

fa4c6836c9366c3cc575cb386a397840d5f1aa57 test: Fix failing time check in rpc_net.py (MarcoFalke)

Pull request description:

  This check fails on slow runners, such as s390x qemu.

  Fix it by using mocktime.

  See https://github.com/bitcoin/bitcoin/pull/28523#discussion_r1357980527

ACKs for top commit:
  0xB10C:
    ACK fa4c6836c9366c3cc575cb386a397840d5f1aa57
  pinheadmz:
    ACK fa4c6836c9366c3cc575cb386a397840d5f1aa57
  brunoerg:
    crACK fa4c6836c9366c3cc575cb386a397840d5f1aa57

Tree-SHA512: 83fb534682e11e97537dc89de8c16f206f38af1a892a2d5970c02684c05eaea8fc9adba3159f16b2440ca0b3871d513a0562a6f3a38f19a5574a47be0919e42f

8cfa22a8464c07da3bb47e7db323ed1069b97457 build: move -fstack-reuse=none to CORE_CXXFLAGS (fanquake)

Pull request description:

  This is not a hardening specific flag, it should be used at all times, regardless of if hardening is enabled or not. Note that this was still the case here, but having this exist in the hardening flags is confusing, and may lead someone to move it inside one of the `use_hardening` blocks, where it would become unused, with `--disable-hardening`.

  Noticed while reviewing https://github.com/hebasto/bitcoin/pull/32#discussion_r1363564161.

ACKs for top commit:
  theuni:
    ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457. Agree it's confusing as-is and this better matches the intent.
  hebasto:
    ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457
  luke-jr:
    utACK 8cfa22a8464c07da3bb47e7db323ed1069b97457
  TheCharlatan:
    ACK 8cfa22a8464c07da3bb47e7db323ed1069b97457

Tree-SHA512: 74c3219301398361d06b1ef2257fc9ec18055b1661f8733ee909adefee61e458d70991c32adf0e0450905a7ffbddc99799f5fdac894f4896cfade19f961818df

2338715506ab99b624ecaf995119d1ace8e4de45 doc: add historical release notes for 25.1 (fanquake)

Pull request description:

  v25.1 has [been tagged](https://github.com/bitcoin/bitcoin/releases/tag/v25.1).
  For merge post-bin-availability.

ACKs for top commit:
  stickies-v:
    ACK 2338715506ab99b624ecaf995119d1ace8e4de45

Tree-SHA512: 799937b8bacab7908d86c91855bfcdfd1f8f806d82df0d04cf984d957d9bf1bbdde6103975358360ef35b0cfeba3ce894bc7dce412ea6790d495b78b3cebee0d

5c8e15c451ec870b9dd4eb843ec6ca3ba64cda4f i2p: destroy the session if we get an unexpected error from the I2P router (Vasil Dimov)
762404a68c114e8831cdfae937627174544b55a7 i2p: also sleep after errors in Accept() (Vasil Dimov)

Pull request description:

  ### Background

  In the `i2p::sam::Session` class:

  `Listen()` does:
  * if the session is not created yet
    * create the control socket and on it:
    * `HELLO`
    * `SESSION CREATE ID=sessid`
    * leave the control socked opened
  * create a new socket and on it:
  * `HELLO`
  * `STREAM ACCEPT ID=sessid`
  * read reply (`STREAM STATUS`), `Listen()` only succeeds if it contains `RESULT=OK`

  Then a wait starts, for a peer to connect. When connected,

  `Accept()` does:
  * on the socket from `STREAM ACCEPT` from `Listen()`: read the Base64 identification of the connecting peer

  ### Problem

  The I2P router may be in such a state that this happens in a quick succession (many times per second, see https://github.com/bitcoin/bitcoin/issues/22759#issuecomment-1609907115): `Listen()`-succeeds, `Accept()`-fails.

  `Accept()` fails because the I2P router sends something that is not Base64 on the socket: `STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"`

  We only sleep after failed `Listen()` because the assumption was that if `Accept()` fails then the next `Listen()` will also fail.

  ### Solution

  Avoid filling the log with "Error accepting:" messages and sleep also after a failed `Accept()`.

  ### Extra changes

  * Reset the error waiting time after one successful connection. Otherwise the timer will remain high due to problems that have been solved long time in the past.

  * Increment the wait time less aggressively.

  * Handle the unexpected "Session was closed" message more gracefully (don't log stupid messages like `Cannot decode Base64: "STREAM STATUS...`) and destroy the session right way.

ACKs for top commit:
  achow101:
    ACK 5c8e15c451ec870b9dd4eb843ec6ca3ba64cda4f
  jonatack:
    re-ACK 5c8e15c451ec870b9dd4eb843ec6ca3ba64cda4f

Tree-SHA512: 1d47958c50eeae9eefcb668b8539fd092adead93328e4bf3355267819304b99ab41cbe1b5dbedbc3452c2bc389dc8330c0e27eb5ccb880e33dc46930a1592885

6889a807661cb570e1464ffdedb9ddb1ec970078 ci: Add missing CI_RETRY_EXE before git clone (MarcoFalke)
b705bade44973e61655d5f847f49d97fb5bb8393 ci: Export `IN_GETOPT_BIN` on macOS (Hennadii Stepanov)
1c2132ddd97dbf8fc908b0d1113bbb70e1a6a64e Revert "ci: Upgrading pip version in macos environment" (Hennadii Stepanov)

Pull request description:

  This PR is a resurrection of https://github.com/bitcoin/bitcoin/pull/28623:

  > This should fix [bitcoin/bitcoin/actions/runs/6457002476/job/17527598426#step:7:240](https://github.com/bitcoin/bitcoin/actions/runs/6457002476/job/17527598426#step:7:240):
  >
  > ```
  > + git clone --depth=1 https://github.com/bitcoin-core/qa-assets /Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets
  > Cloning into '/Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets'...
  > error: RPC failed; curl 18 HTTP/2 stream 5 was reset
  > error: 54975 bytes of body are still expected
  > fetch-pack: unexpected disconnect while reading sideband packet
  > fatal: early EOF
  > fatal: fetch-pack: invalid index-pack output
  > Error: Process completed with exit code 128.
  > ```

ACKs for top commit:
  maflcko:
    lgtm ACK 6889a807661cb570e1464ffdedb9ddb1ec970078

Tree-SHA512: cd50102061a5c57fcf53ecbf552e7eae09995395e91651d00037e3f101c4906c0a994b2f6b1ccf984f2fc902ca59f0db66206ace56a2b6e2810dfa4167f3e115

ec84f999f1408b7f1ff4498f78c33b34c30e934c log: Don't log cache rebalancing in absense of a snapshot chainstate (Fabian Jahr)

Pull request description:

  I have noticed that this log now is always printed, even if there is no snapshot chainstate present or even was present. I think this is confusing to users that have never even thought about using assumeutxo since in that case the rebalancing is just ensuring the normal environment with one chainstate. So I suggest we don't log in absence of a snapshot chainstate. We could also think about rewording the message instead but I think this is simpler.

ACKs for top commit:
  stickies-v:
    utACK ec84f999f1408b7f1ff4498f78c33b34c30e934c
  glozow:
    concept ACK ec84f999f1408b7f1ff4498f78c33b34c30e934c, don't have opinions other than removing confusing log
  theStack:
    utACK ec84f999f1408b7f1ff4498f78c33b34c30e934c

Tree-SHA512: 30bbfc648e7c788106f78d52e47a3aa1e1874f65d13743643dc50bcf7f450d8330711ff9fdeac361722542da6051533153829c6d49033227ed315e111afc899f

dd4dcbd4cda31f67d014a93340a6d1ba1c245b0f [fuzz] Delete i2p target (dergoegge)

Pull request description:

  closes #28665

  The target is buggy and doesn't reach basic coverage.

ACKs for top commit:
  maflcko:
    lgtm ACK dd4dcbd4cda31f67d014a93340a6d1ba1c245b0f
  glozow:
    ACK dd4dcbd4cda31f67d014a93340a6d1ba1c245b0f, agree it's better to delete this test until somebody wants to write a better one

Tree-SHA512: b6ca6cad1773b1ceb6e5ac0fd501ea615f66507ef811745799deaaa4460f1700d96ae03cf55b740a96ed8cd2283b3d6738cd580ba97f2af619197d6c4414ca21

fa21535551e300eaa988d209ad64cdc17fd7f66b fuzz: Increase merge -rss_limit_mb (MarcoFalke)

Pull request description:

  For some reason, the limit is hit. (Presumably due to `-set_cover_merge=1` eating more memory, or by simply having more fuzz inputs).

  Fix it by increasing it for the merge operation.

ACKs for top commit:
  dergoegge:
    ACK fa21535551e300eaa988d209ad64cdc17fd7f66b
  hebasto:
    ACK fa21535551e300eaa988d209ad64cdc17fd7f66b, considering the discussion in https://github.com/bitcoin-core/qa-assets/pull/155.

Tree-SHA512: 4fed0f254eccc6fe0b53656bc345ff898b13811dc39387387317d34b521ab77cee03d82b0896dd92d253b7546b6a7e4bdcd478749f47064374ab44ad759ab9ff

fac36b94ef32567c0f10b605a3a441d11559e56e refactor: Remove CBlockFileInfo::SetNull (MarcoFalke)

Pull request description:

  Seems better to use C++11 member initializers and then let the compiler figure out how to construct objects of this class.

ACKs for top commit:
  stickies-v:
    ACK fac36b94ef32567c0f10b605a3a441d11559e56e
  pablomartin4btc:
    ACK fac36b94ef32567c0f10b605a3a441d11559e56e
  theStack:
    LGTM ACK fac36b94ef32567c0f10b605a3a441d11559e56e

Tree-SHA512: aee741c8f668f0e5b658fc83f4ebd196b43fead3dd437afdb0a2dafe092ae3d559332b3d9d61985c92e1a59982d8f24942606e6a98598c6ef7ff43697e858725

6bdff429ec17eae4138c3af1e21de3ec46f4ab13 build: Include `config/bitcoin-config.h` explicitly in `util/trace.h` (Hennadii Stepanov)

Pull request description:

  The `ENABLE_TRACING` macro is expected to be defined in the `config/bitcoin-config.h` header.

  Therefore, the current code is error-prone as it depends on whether the `config/bitcoin-config.h` header was included before or not.

  This bug was noticed while working on CMake [stuff](https://github.com/hebasto/bitcoin/pull/37).

ACKs for top commit:
  fanquake:
    ACK 6bdff429ec17eae4138c3af1e21de3ec46f4ab13

Tree-SHA512: 22c4fdeb51628814050eb99a83db4268a4f3106207eeef918a07214bbc52f2b22490f6b05fcb96216f147afa4197c51102503738131e2583e750b6d195747a49

fa6588737714cf26571657fc216552a4291376da ci: Add missing --external to podman image prune (MarcoFalke)

Pull request description:

  This should fix the out-of-space issues seen in CI. For example:

  https://cirrus-ci.com/task/6208410429947904?logs=ci#L8613

  ```
  Error: committing container for step {Env:[FILE_ENV=./ci/test/00_setup_env_native_msan.sh PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[bash -c cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh] Flags:[] Attrs:map[json:true] Message:RUN bash -c cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh Original:RUN ["bash", "-c", "cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]}: copying layers and metadata for container "8d882455cc157be6a85d9779b45cacf4dd92a37cfb16fad38213f758a830827d": writing blob: adding layer with blob "sha256:371f657e226fef20f4af6fb88a288dd6248c82c2088daca2d53aaacb51b4303a": processing tar file(write /usr/lib/x86_64-linux-gnu/perl/5.34.0/auto/Unicode/Collate/Collate.so: no space left on device): exit status 1

ACKs for top commit:
  stickies-v:
    utACK fa6588737714cf26571657fc216552a4291376da

Tree-SHA512: a949b957654272b9c9450feebb64397542029019f031a18444b5b403aca899c972b3b163be716bf1bfbd5820430c70a6fec008771c6e13d3d8281ed100df575b

03f82087f6ce1c29327f34d12945200494e6956d doc: assumeutxo prune and index notes (Sjors Provoost)

Pull request description:

  Based on recent comments on #27596.

ACKs for top commit:
  pablomartin4btc:
    re ACK 03f82087f6ce1c29327f34d12945200494e6956d
  ryanofsky:
    ACK 03f82087f6ce1c29327f34d12945200494e6956d. Nice changes, these seem like very helpful notes

Tree-SHA512: fe651b49f4d667400a3655899f27a96dd1eaf67cf9215fb35db5f44fb8c0313e7d541518be6791fec93392df24b909793f3886adb808e53228ed2a291165639d

4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 test: Check tx metadata is migrated to watchonly (Andrew Chow)
d616d30ea5fdfb897f8375ffd8b9f4536ae7835b wallet: Reload watchonly and solvables wallets after migration (Andrew Chow)
118f2d7d70b584eee7b89e58b5cd2d61c59a9bbf wallet: Copy all tx metadata to watchonly wallet (Andrew Chow)
9af87cf3485ce3fac553a284cde37a35d1085c25 test: Check that a failed wallet migration is cleaned up (Andrew Chow)

Pull request description:

  Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain.

  While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added.

ACKs for top commit:
  ishaanam:
    light code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764
  ryanofsky:
    Code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764. Just implemented the suggested orderpos, copyfrom, and path set comments since last review
  furszy:
    ACK 4814e406

Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89

5a0688a20d88a9641c02436abbd7b49e227f1a37 test: enable reindex readonly test on *BSD and macOS as root (Matthew Zipkin)

Pull request description:

  see https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1349505585

  OpenBSD and FreeBSD don't have `chattr` but they do have `chflags`, use that method to make the block file immutable for the reindex_readonly test.

  Written and tested on a VPS running FreeBSD:
  ```
  FreeBSD freebsd-13-1 13.2-RELEASE-p4 FreeBSD 13.2-RELEASE-p4 GENERIC amd64
  ```

ACKs for top commit:
  maflcko:
    re-cr-lgtm-ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37
  jonatack:
    ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37 tested on macOS only
  theStack:
    ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37

Tree-SHA512: 8c88d282d09c00355d22c4c504b779f60e420327a5e07bcf80fa77b97fefcb04952af9ceaf439d9033a0a2448cb26a02663fe6bddcd4a74792857cfbaf1c5162

3f482ac231c3a0077dd236c0ec8f5afc12b71859 doc: add historical release notes for 24.2 (fanquake)

Pull request description:

  v24.2 has [been tagged](https://github.com/bitcoin/bitcoin/releases/tag/v24.2).

ACKs for top commit:
  stickies-v:
    ACK 3f482ac231c3a0077dd236c0ec8f5afc12b71859

Tree-SHA512: 71773832910ecda7ed34c6545d184ecbc743d9a36aadd8e4bd367ff60ef5b8048d39335b2347878c4a1a076cc691f12f0e36e8db542757c883d7f83d1161625d

The negative bound for script threads comes from the machine which
generates the man pages, so may only be correct for that machine. Any
other placeholder value will also be wrong for some machines. Fix this
be removing the value. This also fixes help2man incorrectly bolding the
value, as if it were a paramater.

Closes #28850.

Github-Pull: #28858
Rebased-From: d799ea26edfd63434b6d1cf55500de184dc67fac

compile

1) remove uint8_t quorum types from maps (we only support chainlock quorums)
2) use a new LRU based cache for evodb, reuse across all consuming classes with seperate DBs
3) DMN recreates DB if cache is full as this DB is 1.5gb+ each time and we need to prune to keep it small on disk
4) remove diff usage from IBD as its extra code and faster to just use lists during sync, and goto diffs after that
5) rearrange to remove the ForEachMN loops which are slow, to do only 1 and setup state inside the one (like PoSE banning etc)
6) remove simplifiedmns and remove CB tx having mnlists/merkle root tx, our LC will be Zk based
7) start quorum processing at same time as script sigs are checked to optimize ibd

overall sync is now down to about 20 mins on a reindex from 0 to block 1.85m

9) also fixed assume utxo bug by changing chain params to fix validation manager tests

1) remove llmqType which is unused, we only use chainlock type
2) log only every 2k blocks unless not IBD and debug mode
3) fix tests

remove need for sentinel

only 1 quorum and 10 size of history and the diff with upstream means we can just skip cleanup