🤔 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.

💬 Sjors commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914796771)
> no plans for the interface to support mismatching server/client binaries

That tends to happen without a plan, but indeed we can wait.

> part of the block index, or exposed via RPC

^ this still need to be clarified

👍 TheCharlatan approved a pull request: "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls"
(https://github.com/bitcoin/bitcoin/pull/31490#pullrequestreview-2549773423)
ACK 223081ece651dc616ff63d9ac447eedc5c2a28fa

I don't see a change in the runtime of the benchmarks, but these are some good refactors that I have wanted for some time.

📝 maflcko opened a pull request: " lint: Call more checks from test_runner "
(https://github.com/bitcoin/bitcoin/pull/31653)
The lint `commit-script-check.sh` can not be called from the test_runner at all and must be called manually. Also, some checks require `COMMIT_RANGE` to be set.

Fix all issues by moving two lint checks into the test_runner. Also, the proper commit range is passed to the checks by the test_runner, so that the user no longer has to do it.

💬 marcofleon commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914880590)
I agree that if there were code to keep for now it would be this. I ultimately decided to remove it because the purpose it serves no longer seems to apply if we remove checkpoints. With the headers sync logic, iiuc, it would be fine for a node in IBD to respond to GETHEADERS because the only headers they could have would belong to a chain with enough work. There isn't a risk anymore of sending a bogus chain to a peer and getting disconnected.

This behavior was first introduced in https://gith
...

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2590020502)
I found myself confused by e8b589ef45d5c94c47209fb952e8b05d631a9c47 dropping `AddSocketPermissionFlags`. Can you move that to a separate (earlier?) commit?

💬 marcofleon commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914888912)
I used the same format as the `-upnp` option above. Should be removed soon so probably doesn't matter too much.

💬 marcofleon commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#discussion_r1914890473)
Same thing here, just imitating the `-upnp` format above.

💬 EthanHeilman commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2590077704)
> is this in draft for a reason?

@instagibbs Yes, my todos to make this not a draft are:

[ ] - Better PR description
[ ] - A few extra tapscript tests (it was most cat tapscript tests before)
[ ] - Doublechecking my code to make sure the rebase didn't break anything

I got most of this done over the weekend, I just haven't pushed it yet. Will likely push it tonight, Wednesday