💬 hebasto commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555662120)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.

💬 hebasto commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2555662426)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.

💬 theuni commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2555679006)
What makes NetBSD special in this regard?

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029530)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886502862

> prefer raising `AssertionError` in less roundabout way:

Reason it's written this way is so `-O` or `PYTHONOPTIMIZE` doesn't skip checks this test is trying to perform. (IMO it only makes sense to use `assert` to check for internal assumptions made by test code, which could be skipped without changing coverage provided by the test.)

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029635)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886540011

> Instead of waiting for `addcon_thread_started`, one could wait directly on the asserted content, removing the need to add functionality for yeilding the log content

Reason for doing this is to fit in with existing tests in this file, and to fail right away instead of timing out when the expected log prints are missing. Tests that time out instead of failing are unpleasant to work with.

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893052948)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886811373

I think the comment was accurate but I deleted it now since I already deleted the other comments about this code, too. (The comment was saying that if -noconnect was specified the seednode would be ignored but no "-seednode is ignored when -connect is used" warning would be shown, because `m_specified_outgoing` would be empty. I don't think having the `m_specified_outgoing` condition for that warning makes sense, just li
...

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029815)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886526104

> Could add comment about why this is useful, either in the code or commit ([f6d6383](https://github.com/bitcoin/bitcoin/commit/f6d638331fff8a9b1bd670abe65ee70d1441ad9a))?

Thanks, added comment to explain this.

🤔 ryanofsky reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2516002191)
Thanks for the new review!

Updated 54ad580775b8379a62b05b1f2cdf8b7fde993306 -> 80608ba5d282ae8713ea0136ea9a0208b254c082 ([`pr/listset.5`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.5) -> [`pr/listset.6`](https://github.com/ryanofsky/bitcoin/commits/pr/listset.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/listset.5..pr/listset.6)) improving various comments and adding a new commit to handle `-noasmap` based on Martin's suggestion https://github.com/bitcoin/bitcoin/
...

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029408)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886570537

> nit: Would be nice with a corresponding `else` + check that at least warned if other categories were being mixed together with `0`/`none`.

Definitely agree. I don't think it relates to this PR, but the error checking implemented in this part of code is not great.

💬 ryanofsky commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029305)
re: https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1886587869

> nit: Just seems plain wrong to use `GetArgs` for this parameter if we only allow one value. The only benefit of doing it this way is if one doesn't want to silently overwrite a previous setting value with a following one I guess?

Yes that goes back to introduction of this code in e8990f121405af8cd539b904ef082439261e6c93 from #18267. I don't think this is ideal, but I could also understand allow not wanting to allow
...

💬 TheCharlatan commented on pull request "test: Embed univalue json tests in binary":
(https://github.com/bitcoin/bitcoin/pull/31542#issuecomment-2555691218)
Nice, Concept ACK

💬 hebasto commented on pull request "test: Embed univalue json tests in binary":
(https://github.com/bitcoin/bitcoin/pull/31542#issuecomment-2555699712)
Concept ACK. Thank you for picking this up!

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1893147509)
xcb libraries are part of the X system libraries, everyone running X11 (or xwayland) on linux will have them, someone not running that isn't going to run bitcoin-qt
there's no problem here

💬 maflcko commented on pull request "[RFC] Align debugging flags to `-O0`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2555811602)
The CI failure:

```
[08:34:27.775] ********* Start testing of WalletTests *********
[08:34:27.775] Config: Using QtTest library 5.15.14, Qt 5.15.14 (i386-little_endian-ilp32 static debug build; by GCC 13.2.0), ubuntu 24.04
[08:34:27.775] PASS : WalletTests::initTestCase()
[08:34:27.775] QDEBUG : WalletTests::walletTests() NotifyUnload
[08:34:27.775] QWARN : WalletTests::walletTests() This plugin does not support propagateSizeHints()
[08:34:27.775] QDEBUG : WalletTests::walletTests()
...

💬 Torabora33 commented on issue " (bitcoin core warning: unknown new rules activated versionbit 2) ":
(https://github.com/bitcoin/bitcoin/issues/31536#issuecomment-2555827704)
It already finished Sync i restart the pc couple of times but still same warning. is there any video i can watch to do the reindexing my macbook?

i downloaded the newest version of bitcoin core wallet and run it and it finished sync but no balance showing up there either
only i don't see the waring to the newest version of bitcoin core wallet showing up.... this is making me kinda dizzy

💬 hebasto commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2555845321)
> Edit: And where is your `libsqlite3.so.0` that it can't find?

As a result of executing the `pkgin install sqlite3` [command](https://github.com/bitcoin/bitcoin/blob/master/doc/build-netbsd.md#descriptor-wallet-support), the library is installed in the `/usr/pkg`:
```
$ ls -1 /usr/pkg/lib/libsqlite*
/usr/pkg/lib/libsqlite3.a
/usr/pkg/lib/libsqlite3.la
/usr/pkg/lib/libsqlite3.so
/usr/pkg/lib/libsqlite3.so.0
/usr/pkg/lib/libsqlite3.so.0.8.6
```

> What makes NetBSD special in this re
...

👍 TheCharlatan approved a pull request: "test: Embed univalue json tests in binary"
(https://github.com/bitcoin/bitcoin/pull/31542#pullrequestreview-2516311556)
ACK fabda022536c7a000a575bbb05059d27d29b5cca

💬 l0rinc commented on pull request "refactor: cache block[undo] serialized size for consecutive calls":
(https://github.com/bitcoin/bitcoin/pull/31490#issuecomment-2555874634)
@maflcko, @theuni, the reindex and IBD didn't show any speedup, so I've added a `SaveBlockToDiskBench` microbenchmark which only revealed a tiny speedup in itself, so I've demoted this from an optimization to a refactoring.
This PR is now meant as a cleanup for follow-ups such as https://github.com/bitcoin/bitcoin/pull/31539 and https://github.com/bitcoin/bitcoin/pull/31144 where the total IBD speedups (until 870k blocks) add up to 9% - and `ReadBlockFromDiskTest` benchmark is 2x faster.

<de
...

📝 hebasto opened a pull request: "cmake: Remove unused `BUILD_TESTING` variable from "dev-mode" preset"
(https://github.com/bitcoin/bitcoin/pull/31544)
On the master branch @ bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd:
```
$ cmake -B build --preset dev-mode -DWITH_MULTIPROCESS=OFF
<snip>
-- Configuring done (12.0s)
-- Generating done (0.1s)
CMake Warning:
Manually-specified variables were not used by the project:

BUILD_TESTING


-- Build files have been written to: /home/hebasto/git/bitcoin/build
```

This PR resolves the issue.

The removed `BUILD_TESTING` variable is a part of the [`CTest`](https://cmake.org/cmake/help/l
...

💬 jstefanop commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2555984586)
> I guess we should save a "reindex needed" flag somewhere when LevelDB corruption causes the node to shutdown?
>
> > [I wish I could re-download only the blocks since then, not have to re-download the entire chain again.](https://bitcoin.stackexchange.com/q/123059/4334)
>
> LevelDB corruption means you have a bad chainstate. It's not really possible to rebuild it without a full reindex from genesis, short of us doing something like utreexo commitments (even without a softfork, we could se
...