💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1824345643)
Please disregard my previous comments.

I've reviewed and tested the current branch once more. Modifications in `contrib/guix/libexec/build.sh` are not required. Setting proper compilers is handled already:
- for the `native_qt` package: https://github.com/bitcoin/bitcoin/blob/af05dd9a12b89224dc7ad229698eeceb3e560ed4/depends/packages/native_qt.mk#L88-L89
- for other CMake-based native packages: https://github.com/bitcoin/bitcoin/blob/af05dd9a12b89224dc7ad229698eeceb3e560ed4/depends/funcs.mk#
...

👍 stickies-v approved a pull request: "[27.x] rc2 or final"
(https://github.com/bitcoin/bitcoin/pull/31154#pullrequestreview-2407773071)
ACK dd1bf8bc4ef75ef43c33bbf755c3e0d2c6c3c5f7

Doc-only change, I'm getting the same manpages results.

💬 maflcko commented on pull request "Revert "Introduce `g_fuzzing` global for fuzzing checks"":
(https://github.com/bitcoin/bitcoin/pull/31189#issuecomment-2449710201)
It would be good to make it `constexpr` instead https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449665430 and use that to fix the issues. Happy to do it myself, but I think it should be part of one pull request. Otherwise you'll just switch one issue for another and a follow-up has to be created anyway.

💬 dergoegge commented on pull request "Revert "Introduce `g_fuzzing` global for fuzzing checks"":
(https://github.com/bitcoin/bitcoin/pull/31189#issuecomment-2449717790)
I don't think the approach described in https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2449665430 would fix the ci issues? If we make it a compile time option we'll see the timeouts in `p2p_headers_presync` for the macos and windows CI either way.

💬 hodlinator commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1824361387)
Nothing more to do as far as this PR goes. Still interested to hear from folks more focused on fuzzing.

💬 maflcko commented on pull request "Revert "Introduce `g_fuzzing` global for fuzzing checks"":
(https://github.com/bitcoin/bitcoin/pull/31189#issuecomment-2449726713)
Yeah, I am saying that the fix should be a complete one, at least when it comes to the `./src` directory. Otherwise it will have to be touched again. I am happy to submit a pull, or review one. But I think there have been enough issue and pull request threads about this topic and it would be good to wrap it up in at most one or two pulls.

✅ dergoegge closed a pull request: "Revert "Introduce `g_fuzzing` global for fuzzing checks""
(https://github.com/bitcoin/bitcoin/pull/31189)

💬 dergoegge commented on pull request "Revert "Introduce `g_fuzzing` global for fuzzing checks"":
(https://github.com/bitcoin/bitcoin/pull/31189#issuecomment-2449729047)
> I am happy to submit a pull

Yes please

👋 fanquake's pull request is ready for review: "[27.x] rc2 or final"
(https://github.com/bitcoin/bitcoin/pull/31154)

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2449751980)
The CI failure in https://github.com/bitcoin/bitcoin/runs/32217592364 might come from a bad rebase reconciliation with master?
```
[12:44:30.723] Duplicate include(s) in src/bench/xor.cpp:
[12:44:30.723] #include <cstddef>
[12:44:30.723] #include <span.h>
[12:44:30.723] #include <streams.h>
[12:44:30.723] #include <random.h>
[12:44:30.723] #include <vector>
[12:44:30.723] #include <bench/bench.h>
[12:44:30.723]
[12:44:30.728] ^---- ⚠️ Failure generated from lint-includes.py
```

📝 glozow opened a pull request: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190)
Addresses some remaining followups from #30110:
- https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1818893833
- https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1819638959
- https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1819634235

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1824397333)
picked up in #31190

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1824397572)
added in #31190

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1824398039)
Agree the comment isn't 100% accurate, edited in #31190

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1824400387)
since txdownloadman is locked with this mutex, and it's the only one with access to these data structures, we have the same locking happening. Since `m_tx_download_mutex` is external, we wouldn't be able to have these annotations I think

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2449786517)
> The CI failure in [bitcoin/bitcoin/runs/32217592364](https://github.com/bitcoin/bitcoin/runs/32217592364) might come from a bad rebase reconciliation with master?

That's an old build, right? Latest [Lint](https://github.com/bitcoin/bitcoin/pull/31144/checks?check_run_id=32217909313) seems fine

📝 maflcko opened a pull request: " Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz"
(https://github.com/bitcoin/bitcoin/pull/31191)
`g_fuzzing` is used inside `Assume` at runtime, causing significant overhead in hot paths. See https://github.com/bitcoin/bitcoin/issues/31178

One could simply remove the `g_fuzzing` check from the `Assume`, but this would make fuzzing a bit less useful. Also, it would be unclear if `g_fuzzing` adds a runtime overhead in other code paths today or in the future.

Fix all issues by making `G_FUZZING` equal to the build option `BUILD_FOR_FUZZING`, and for consistency in fuzzing, require it to
...

👍 dergoegge approved a pull request: "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz"
(https://github.com/bitcoin/bitcoin/pull/31191#pullrequestreview-2407916901)
utACK fafbf8acf419d5e2ca307e5804099361ca7471af

💬 dergoegge commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#discussion_r1824434342)
```suggestion
if constexpr (IS_ASSERT || std::is_constant_evaluated() || G_FUZZING
```

💬 fanquake commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824438854)
You can keep the std lib headers separated from our own headers.