👍 stickies-v approved a pull request: "kernel: Move background load thread to node context"
(https://github.com/bitcoin/bitcoin/pull/30896#pullrequestreview-2303572995)
ACK 3a26c839c63ecda2d3b2491d6d84a03c1d8f508f

I looked into the history of why `m_thread_load` is a `ChainstateManager` member, but it appears there's no particular reason. faf843c07f99f91603e08ea858f972516f1d669a deglobalized `g_load_block` into `ChainstateManager::m_load_block` (later renamed (04575106b2529f495ce8110ddf7ed2247d4bc339) to `m_thread_load`) and it appears the clean-up of this PR could have been applied to that commit too (but building that >3yo commit seems non-trivial so I can
...

💬 stickies-v commented on pull request "kernel: Move background load thread to node context":
(https://github.com/bitcoin/bitcoin/pull/30896#discussion_r1759167497)
nit: needs Doxygen cross-reference update in `developer-notes.md` to reflect the new anchor `structnode_1_1_node_context.html#aef02af873199206a8f032a3917277bef` (as per `cmake --build build -t docs` on 3a26c839c63ecda2d3b2491d6d84a03c1d8f508f)

<details>
<summary>git diff on 3a26c839c6</summary>

```diff
diff --git a/doc/developer-notes.md b/doc/developer-notes.md
index a630957f41..febd158c68 100644
--- a/doc/developer-notes.md
+++ b/doc/developer-notes.md
@@ -634,7 +634,7 @@ Threads

...

💬 brunoerg commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349471125)
> I briefly fuzzed after making the change and didn't hit anything. Likely I just didn't fuzz enough, but posting just in case somebody has a fuzz output that I could use to verify.

I left different mutations of it running overnight (12h+) and I also didn't get a crash for this one.

> I think it would be better not to add anything like the above because one might fuzz bitcoind directly, e.g. we could consider dropping the custom honggfuzz netdriver patch and use FUZZING_BUILD_MODE_UNSAFE_F
...

💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349476460)
> I briefly fuzzed after making the change and didn't hit anything. Likely I just didn't fuzz enough, but posting just in case somebody has a fuzz output that I could use to verify.

@glozow thanks for reviewing. I'm looking into this more. Good catch actually. Turns out I only partially reverted that commit when testing (only moved the `return true` out of the `else` in `TryLowWorkHeadersSync`). In that case, the fuzzer crashes on this line:
https://github.com/bitcoin/bitcoin/blob/e43ce250c6
...

💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349486479)
> I left different mutations of it running overnight (12h+) and I also didn't get a crash for this one.

Good to know, thanks. I'll mark this PR as a draft for now until I figure what's going on.

📝 marcofleon converted_to_draft a pull request: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661)
This PR reopens https://github.com/bitcoin/bitcoin/pull/28043. It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355 and [a couple bugs](https://github.com/bitcoin/bitcoin/pull/25717/commits/ed6cddd98e32263fc116a4380af6d66da20da990) that were addressed in https://github.com/bitcoin/bitcoin/pull/25717. This should help us move forward with the [removal of mainnet checkpoints](https://github.com/bitcoin/bitcoin/pull/25725).

It seems like the main concern in https://githu
...

👍 instagibbs approved a pull request: "cluster mempool: optimized candidate search"
(https://github.com/bitcoin/bitcoin/pull/30286#pullrequestreview-2303655329)
reACK 9ad2fe7e69e9e69949ebbb280a15756dc3301f09

just a rebase on master to take up `Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"` I presume

via `git range-diff master a5ea6467b6a694facc1f5efe460bf30539167597 9ad2fe7e69e9e69949ebbb280a15756dc3301f09`

⚠️ 1440000bytes opened an issue: "Use IPv4-encoded IPv6 address to get IPv4 node address with port number from DNS seeds"
(https://github.com/bitcoin/bitcoin/issues/30900)
### Please describe the feature you'd like to see added.

Use the IPv4-encoded IPv6 addresses that start with a reserved prefix to support nodes with non-default ports in bootstrapping process.

There are 7k IPv4 reachable nodes and some of them use non-default ports:


Example: Port 39388 is used by more than [1000 nodes](https://bitnodes.io/nodes/all/ports/)

The scripts used in this testing guide
...

💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1759245113)
> sighash value does not match

That's a bug I think, but I'm not sure what's causing it.

🤔 jonatack reviewed a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875#pullrequestreview-2303730050)
Concept ACK

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759264704)
```suggestion
was in the active chain, as would be the case after calling the `invalidateblock` RPC.
```

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759260020)
```suggestion
On Unix systems, the `-DWITH_MULTIPROCESS=ON` build option can be passed to build the supplemental `bitcoin-node` and `bitcoin-gui` multiprocess executables.
```

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759267278)
```suggestion
print("Recompile with the -DBUILD_DAEMON=ON build option")
```

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759265438)
```suggestion
2. Follow the compile instructions for your system, adding the `-DCMAKE_BUILD_TYPE=Debug` build flag
```

💬 jonatack commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349722686)
This brings back memories when contributing to Ruby on Rails, that `skip ci` was requested to be used in any doc-only changes.

No strong opinion, but when opening or updating a PR that only touches a markdown file, I'd use this.

💬 achow101 commented on pull request "qt: Translations update":
(https://github.com/bitcoin/bitcoin/pull/30899#issuecomment-2349757169)
ACK ae0529576147a1a5bee992574e2cefc8a1fa37d0

👍 tdb3 approved a pull request: "test: Check already deactivated network stays suspended after dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/30892#pullrequestreview-2303789215)
tested ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa

Great addition.
Added improper `node.setnetworkactive()` lines above each call to `self.check_expected_network()` to ensure the checks would fail. They did (no surprise).

💬 hebasto commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1759331776)
Why does referencing `../src/ipc/capnp/` not work from `src/test/CMakeLists.txt`?

💬 achow101 commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2349919694)
ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5

🚀 achow101 merged a pull request: "refactor: move m_is_inbound out of CNodeState"
(https://github.com/bitcoin/bitcoin/pull/30233)