💬 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)

💬 tdb3 commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#discussion_r1731952540)
nit: Adjusted this in the [Wiki Page](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft). Line wrap issue. Also moved to P2P section.

💬 tdb3 commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#discussion_r1731954716)
nit: Adjusted this in the [Wiki Page](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft). Line wrap issue.

💬 luke-jr 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#discussion_r1731977769)
Mine is here if you want to add it:

https://luke.dashjr.org/programs/bitcoin/files/charts/seeds.txt

💬 luke-jr commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2311352168)
Any reason not to just lock the mutex across the three steps?

💬 luke-jr commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#discussion_r1731984776)
Since status stops working when the scan completes, it seems like we should clear `g_relevant_blocks` here?

💬 luke-jr commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#issuecomment-2311368646)
>Bump to 28.99 in preparation for the 28.x branching

This should be post-branching...?

💬 tdb3 commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#discussion_r1732046737)
Yes, thank you. Will update.

💬 achow101 commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#issuecomment-2311499781)
> This should be post-branching...?

I don't think it particularly matters since that step has to be done in the 28.x branch too, in addition to the other version numbers being bumped.