🤔 furszy reviewed a pull request: "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface"
(https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2261512268)
self-code-ACK

💬 achow101 commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1731757103)
In a2317e27b7fb86df4e32cd1674c06e09cb808248 "cmake: Add root `CMakeLists.txt` file"

The plan is to have this merged post 28.x branch off, so I think this should be set to 28.

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2311026632)
Last week I was [hit by this again](https://github.com/hodlinator/bitcoin/actions/runs/10491632082/job/29061229925). Probably line ending differences on Windows makes any additional Python exception reporting extra annoying to read due to every second line being blank (separate issue, but slightly improved by this PR producing less output).

### Git-sleuthing to help motivate this PR
#### Was this ever working?
Unfortunately for my case, it seems the answer is no:
7897338918dac072e788b8ab29
...

💬 theuni commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2311036181)
x86-64 Guix build:
```
bfce0ebd120bd45576586691c46b47e66bb0031f7540772a924d4cb1c2602576 aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu-debug.tar.gz
615aae82e503fc07ee1053649b3a45ad2f0dd9c02d5a85394f8ba33b3bd5be88 aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu.tar.gz
c4e0333106b259d1ab11066677fc32fba8dd380afc2f9c334d9eff2dccccba39 aarch64-linux-gnu/SHA256SUMS.part
152a84bd68011d9af6e30830f4da1a72ebee401160c1fefd46e9de0cae77c94a arm64-apple-darwin/bitcoin-41051290ab3b-
...

💬 achow101 commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2311072370)
doc/developer-notes.md needs to be updated too.

💬 ryanofsky commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1731810191)
In commit "init: fix fatal error on '-wallet' negated option value" (0df52e24363f729210887ca20a47824ba8073617)

I think this should be extended to test `-nowallet=0`. Maybe consider making this a loop to test both `-nowallet=0` `-nowallet=not_a_boolean`.

Reason: the simplest and most direct way to trigger the uncaught exception is to write `-nowallet=0`. The `-nowallet=not_a_boolean` case is a more complicated and indirect way to trigger it relies on `IntepretBool` interpreting the string `
...

👍 ryanofsky approved a pull request: "init: fix init fatal error on invalid negated option value"
(https://github.com/bitcoin/bitcoin/pull/30684#pullrequestreview-2261617194)
Code review ACK 0df52e24363f729210887ca20a47824ba8073617

💬 jonatack commented on pull request "[WIP] seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes":
(https://github.com/bitcoin/bitcoin/pull/30695#issuecomment-2311114681)
> your cjdns node is no longer part of the cjdns node list because the node didn't have the NODE_NETWORK version bit set

Yes, I reluctantly had to begin pruning due to the increased rate of chain data growth. Is there now only one CJDNS node?

💬 brunoerg commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311120308)
Concept ACK

💬 furszy commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1731849835)
absolutely. Pushed.

💬 jonatack commented on pull request "[WIP] seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes":
(https://github.com/bitcoin/bitcoin/pull/30695#issuecomment-2311138379)
> Also, how do you define very good nodes?

I was manually curating I2P nodes based on trusted colleagues (akin to addnode peer selection), filtered by connection reliability and regularly seeing blocks from them. You have some that I'd recommend and only a couple that seem missing, so not an issue. Feel free to contact me via IRC to discuss if you like.

💬 hodlinator commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311170636)
Would something like this be useful in helping approach the root cause of this?
```diff
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -5,6 +5,7 @@
"""Class for bitcoind node under test"""

import contextlib
+from collections import Counter
import decimal
import errno
from enum import Enum
@@ -260,6 +261,7 @@ class TestNode():
"""Sets up an RPC connection to the bitcoind process. Returns False if unable to connec
...

💬 achow101 commented on issue "An "output descriptor" should not have many different checksums":
(https://github.com/bitcoin/bitcoin/issues/30632#issuecomment-2311176140)
@Rob1Ham That is an unrelated issue likely caused by usage of single quotes for the hardened identifier (notice how your import string uses a single quote). I suggest using `h`, it resolves a ton of shell escape issues that can cause errors like that.

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2311186892)
Another argument for this PR is that even the current title of #30390 is "ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it". This in itself when looking at logs like https://github.com/bitcoin/bitcoin/actions/runs/9677128783/job/26698120754#step:27:225 is actually just a knock-on exception happening after the RPC connection attempt times out and `stop()`-RPC is called in vain:
<details>
<summary>Copied log excerpt</summary>


...

💬 Rob1Ham commented on issue "An "output descriptor" should not have many different checksums":
(https://github.com/bitcoin/bitcoin/issues/30632#issuecomment-2311224515)
> @Rob1Ham That is an unrelated issue likely caused by usage of single quotes for the hardened identifier (notice how your import string uses a single quote). I suggest using `h`, it resolves a ton of shell escape issues that can cause errors like that.

Confirmed using hs worked!

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2311246143)
Rebased due to #30690

💬 luke-jr commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2311259644)
Initial testing: Seems to work fine. I was surprised Qt wasn't at least enabled by default.

> The "auto" value is no longer available; non-default values must be specified explicitly.

I assume this is a CMake standard practice? Or is there some other reason to not autodetect bdb/portmapping/qt/zmq?

💬 luke-jr commented on pull request "validation: improve m_best_header tracking":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2311302788)
>It also suggest to mark headers between an invalidatetd block and the previous m_best_header as invalid, so they won't be considered in the recalculation.

You mean if you invalidateblock, then reconsiderblock, the rest of the child blocks also need to be reconsidered individually? That seems like a conceptual bug IMO, backward incompatible, and quite annoying.

💬 luke-jr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#issuecomment-2311323207)
Maybe we should have a custom locator with the last-change block, and last-scanned block?

👍 tdb3 approved a pull request: "Pre-28.x branch off version bump and doc updates"
(https://github.com/bitcoin/bitcoin/pull/30719#pullrequestreview-2261845132)
ACK b58869f3bf5a468ef96152d357d0f8bdb7cce07d
Left nits, but resolved them in the [Wiki page](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft)