💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2074934165)
> Please delete the hidden comment in the pull description, because this will end up in the merge commit.

Oh, sorry that. Deleted.

💬 fanquake commented on issue "Require thread_local":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074938316)
Possibly. Someone will first need to test it on FreeBSD and mingw-w64. Given we still assume those implementations to be broken.

✅ fanquake closed an issue: "SEE ALSO section for all man pages"
(https://github.com/bitcoin/bitcoin/issues/29558)

🚀 fanquake merged a pull request: "contrib: list other binaries in manpage output"
(https://github.com/bitcoin/bitcoin/pull/29585)

💬 maflcko commented on issue "Require thread_local":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2074959708)
I couldn't get wine running locally, so my testing failed. But given that we require a C++20 compiler, I'd be surprised if there is still a platform out there that hasn't fully implemented C++11 at this point.

I presume, at least for the mingw issue, the existing unit tests should detect the issue, as I presume the `RenameEnMasse` unit test is equivalent to the C++ shared in the linked gist?

🤔 kevkevinpal reviewed a pull request: "test: Run framework unit tests in parallel"
(https://github.com/bitcoin/bitcoin/pull/29771#pullrequestreview-2019885446)
Approach ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d

I still need to run this on my local machine

💬 kevkevinpal commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1577898651)
is there any reason we picked this location in `BASE_SCRIPTS`, I would think we would want to run `TEST_FRAMEWORK_UNIT_TESTS` earlier on even if they are running in parallel

since if the unit tests fail other tests will probably fail as well.

I think making it the first script in `BASE_SCRIPTS` would make sense to me and then making an exception in regards to run times since other tests deepened on this.

👍 ryanofsky approved a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2019819063)
Code review ACK 3c3895ccfa108e10f4a0b1a78f64ffedade1fd11

Left another suggestion, but this looks good in its current form.

💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577858964)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)

This might be a little simpler and clearer since it consolidates the best header chain cases:

```diff
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5292,15 +5292,14 @@ void ChainstateManager::CheckBlockIndex()
pindex = range.first->second;
nHeight++;
continue;
- } else if (best_hdr_chain.Contains(pindex) && nHeight < best_hdr
...

💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1577887301)
In commit "validation: improve performance of CheckBlockIndex" (7183bc90483cc9361cfffb157b91fd4245ab77e0)

Note: this assert is a little different than what I suggested https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1569423672 because it is not enforcing that if the parent block is the best header, then pindex must be null. So the statement that there will not be a best block if the parent block is the best header is not checked by the assert.

But I think this is fine. The commen
...

💬 maflcko commented on pull request "ci: disable `_FORTIFY_SOURCE` with MSAN":
(https://github.com/bitcoin/bitcoin/pull/29837#issuecomment-2075003021)
lgtm ACK 08ff17d1420a3d1c14c6b1a5436678fbb1dd9cbc

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577936862)
I don't think that is much simpler. Also I think this is easier to delete if/when we don't try orphans from different peers in the future.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577947487)
Ok, marking this as resolved

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577949702)
Added a similar test

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1577950806)
Added

💬 hebasto commented on pull request "refactor: Rename `subprocess.hpp` to follow our header name conventions":
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2075053593)
> > Resolved linter warnings.
>
> Can you mention this in the PR description?

Thanks! The PR description has been updated.

👍 instagibbs approved a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2020006914)
ACK https://github.com/bitcoin/bitcoin/pull/28970/commits/30c9e6bc4e9f8b8b606e55435dc3f743cb2dd670

Changes are test only, and new subtest causes a debug-build crash when the `PCKG_POLICY` change is reverted.

💬 iotamega commented on issue "Crashes in v27 on Windows 10 and 11":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075063698)
Hate to be the deliverer of bad news but I am also experiencing this issue on my Ubuntu installations after extensive testing.

💬 maflcko commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2075073807)
On Ubuntu it should be trivial to run in gdb, can you try that please?

👍 TheCharlatan approved a pull request: "refactor: Rename `subprocess.hpp` to follow our header name conventions"
(https://github.com/bitcoin/bitcoin/pull/29910#pullrequestreview-2020030582)
ACK 08f756bd370c3e100b186c7e24bef6a033575b29