👍 brunoerg approved a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2549447462)
code review ACK 2ed161c5ce648cb66ec3d2941b02d68b6ca4c390

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914669735)
I was wondering if these are stored as part of the block index, or exposed via RPC. Otherwise changing their numeric values shouldn't matter.

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914673933)
Indeed seems better to keep this. Perhaps mention the last mainnet checkpoint height, since `MinimumChainWork()` is far above that.

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914677090)
Judging from the change to `ipc_test.cpp` below it seems that eventually the values might be sent over IPC. At that point we have to fix their integer values. Might as well do it now? cc @ryanofsky

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914687073)
This is a nice improvement. I've been running my node with `-nocheckpoints=1` for years, so now it will just throw a warning rather than abort launch. (which I tested)

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#issuecomment-2589713954)
Concept ACK

If we go ahead with this, right after the v29 branch-off would seem like a good time.

I plan to expand the alternate mainnet chain in #31583 to 11111+ blocks and have ~p2p_dos_header_tree.py` use that instead. That would make it easier to revert the checkpoints even after testnet3 is completely dropped.

💬 hebasto commented on pull request "depends, NetBSD: Fix `bdb` package compilation with GCC-14":
(https://github.com/bitcoin/bitcoin/pull/31613#issuecomment-2589737954)
> It looks like `string.h` includes `strings.h` which defines these functions, but only if `_NETBSD_SOURCE` is defined. But I'm not sure who's supposed to define that? I suspect that adding it to our cppflags _might_ be the correct thing to do, but who knows what side-effects it would have.

Defining `_NETBSD_SOURCE` works, but I agree that it is not an optimal solution, especially when the `_XOPEN_SOURCE` is already defined.

> Also, it seems these were masked by the fact that we use `-Wno-
...

💬 kevkevinpal commented on pull request "test: avoid internet traffic":
(https://github.com/bitcoin/bitcoin/pull/31646#issuecomment-2589756582)
light code review ACK [2ed161c](https://github.com/bitcoin/bitcoin/pull/31646/commits/2ed161c5ce648cb66ec3d2941b02d68b6ca4c390)

I think it makes sense that our test suite should not attempt to make outbound connections this is a welcomed change

🤔 marcofleon reviewed a pull request: "fuzz: Abort when global PRNG is used before SeedRand::ZEROS"
(https://github.com/bitcoin/bitcoin/pull/31548#pullrequestreview-2549561974)
ACK fa3c787b62af6abaac35a8f0d785becdb8871cc0

iiuc this PR adds a check to make sure that no randomness is used before `SeedRandomStateForTest(SeedRand::ZEROS)` occurs, vs what `check_globals` does which is just checking if seeding happened at any point in the fuzz test.

Tested by moving `SeedRandomStateForTest(SeedRand::ZEROS)` to after test setup in `utxo_total_supply` and saw the assertion failure.
```
/src/test/util/random.cpp:45 SeedRandomStateForTest: Assertion `!g_used_g_prng' fail
...

💬 Sjors commented on pull request "ci: Turn CentOS task into native one":
(https://github.com/bitcoin/bitcoin/pull/31651#issuecomment-2589771910)
Tested on my self-hosted Cirrus setup, seems fine: https://cirrus-ci.com/task/6297483418009600

👍 BrandonOdiwuor approved a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2549577230)
Code Review ACK 2ed161c5ce648cb66ec3d2941b02d68b6ca4c390

🤔 Mohammed-Alanazisa reviewed a pull request: "depends: Fix `CXXFLAGS` on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/31502#pullrequestreview-2549579161)
gh pr checkout 31502gh

📝 Mohammed-Alanazisa opened a pull request: "Create devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/31652)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ hebasto closed a pull request: "Create devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/31652)

📝 hebasto locked a pull request: "Create devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/31652)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Sjors commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1914754381)
This does make the debug level even noisier. But since it doesn't have the word "disconnect" it's probably fine.

💬 Sjors commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#issuecomment-2589826410)
utACK 286de9749612565fd8d42f08f66c8426faf8a1f7

💬 vasild commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1914779564)
I removed this because the same options were enabled by CMake's HARDERNING option when in Debug mode, so this became redundant. However, later I changed HARDENING+Debug to only use `_GLIBCXX_ASSERTIONS` instead of `-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC` because the latter produced: `undefined reference to mp::Connection::removeSyncCleanup(__gnu_debug::_Safe_iterator...`.

Maybe this should stay. I wonder why using `-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC` from here does not produce an
...

💬 vasild commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1914780635)
Interesting. Is that using libstdc++ or libc++?

💬 maflcko commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914788340)
> these values will be sent over IPC. At that point we have to fix their integer values. Might as well do it now?

Why would they need to be fixed? IPC is only an internal interface. It is currently experimental and in draft and there are no plans for the interface to support mismatching server/client binaries. I'd say the code should be left as-is until there are plans (with support) to even support that. Until then, changing the code for that reason seems like pointless churn to me.