💬 Sjors commented on pull request "Clear out space on GHA jobs":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3381040718)
> Happy to revert this to only apply to the CentOS job if preferred. Seems pointless to add 10 mins runtime onto like 11 jobs where it's not needed...

While testing on with `sv2-tp` https://github.com/Sjors/sv2-tp/pull/37 I also noticed the cleanup task is quite slow.

⚠️ MRHUNG369 opened an issue: "bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33572)

✅ fanquake closed an issue: "bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33572)

💬 hebasto commented on pull request "[wip] A more static bitcoin-qt":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3381076578)
> > How is this approach better than #29923?
>
> They are doing different things. This PR produces a (much more) static `bitcoin-qt`, using depends as is. #29923 introduces new tooling, to essentially "hide" the runtime loading, the binary is not any more static.

Sure. But #29923 also reduces build dependencies. Isn’t that one of the goals of build system design?

💬 Sjors commented on pull request "ci: Add macOS cross task for arm64-apple-darwin":
(https://github.com/bitcoin/bitcoin/pull/33549#issuecomment-3381079356)
utACK fad5a7101cc3dccbb525cfe9afc105aace8da88e

CI log suggests it worked.

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2413540219)
Think this is because the child processes have the same `g_rng_temp_path` so they create the same directory. Will need to see if `GetStrongRandBytes` introduces non-determinism. I'm confused why fuzzing on debian didn't bring this up.

💬 hebasto commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3381089487)
> Extends [7703884](https://github.com/bitcoin/bitcoin/commit/7703884ab19cd7ffddc5c52ba57dec82fbc8dc2b) to guard environ declaration on all Windows builds, not just MSVC.
> MinGW also defines environ as a macro that expands to a dllimport function, causing the same inconsistent linkage warning.

I suppose this behaviour is specific to UCRT.

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2413568289)
I think there is potential for unconditional logging abuse here. If an inbound peer (which doesn't get disconnected for sending `BLOCK_CACHED_INVALID` headers) knows of (or spends the PoW to construct) an invalid block with a valid header, they can resend the same header and spam the log. The impacts should be limited since we now have disk filling mitigation in place, but it's still annoying at the very least, and it's very cheap to execute on the entire network when e.g. a miner accidentally c
...

💬 fanquake commented on pull request "[wip] A more static bitcoin-qt":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3381125205)
> But https://github.com/bitcoin/bitcoin/pull/29923 also reduces build dependencies.

I'm not sure it reduces them; they are just moved into a different repository? That repository will need ongoing maintenance, the deps (headers) copied into there, as well as a [fake libc](https://github.com/laanwj/qtsowrap/tree/main/scripts/fake_libc_include), still need updating (I'm assuming syncronised with the Qt we are shipping in this repo, so changes needed there could also block changes here), as wel
...

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413596564)
This would require `m_workers` to be synchronized somehow, since it is written in `Stop` and `Start` but read in `Submit`.

👋 naiyoma's pull request is ready for review: "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value"
(https://github.com/bitcoin/bitcoin/pull/33498)

💬 fanquake commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381316115)
> but it seems this is a workaround for an upstream bug?

Yep, looks like upstream has confirmed it's an upsteam bug: https://github.com/mstorsjo/llvm-mingw/issues/522#issuecomment-3381159016.

💬 maflcko commented on pull request "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found":
(https://github.com/bitcoin/bitcoin/pull/33433#discussion_r2413754626)
oh, I got the logic exactly reversed

💬 maflcko commented on pull request "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found":
(https://github.com/bitcoin/bitcoin/pull/33433#issuecomment-3381368024)
review ACK 79b4c276e7b9b526fa8f563b1e09b2b970baece6 🏑

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 79b4c276e7b9
...

💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413787797)
Is there value in making the `ThreadPool` safe for recursive task submission? We might not be able to implement certain divide and conquer algorithms, but we might not need to.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3381463897)
Rebased 64d1449ff47dcb2d51a25f4178845538061c8644 -> 2f6d5637d8cd1dc1a1444e31fdfb2dfb13152500 ([kernelApi_69](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_69) -> [kernelApi_70](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_70), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_69..kernelApi_70))


* Fixed silent merge conflict with #32998.
* Some smaller improvements to the c++ header.

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413851837)
I don't think so. In that case, should your test have some warnings that this can be unsafe if the thread pool is stopped or destroyed before all calls to `Submit` have returned?

💬 hodlinator commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3381560071)
I hate spam too. Thanks @hsdredgun for running a node.

> this change is catastrophic.

No. Data can already be encoded in witness data at a ~4x discount compared to OP_RETURN.

> Fix the witness discount problem.

Not sure what you are referring to here, changes like #28408 lead to an endless cat & mouse game. Spammers can release new spam-encoding schemes faster than new node software can be released. Even if we had some kind of frequently updated dynamic filtering mechanism that was a
...

💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413885650)
We can modify the test to make the parent task wait for the child task to complete before returning. `Stop` waits for all works to complete before clearing the workers vector. That is, if @furszy determines that there is any point to a recursive test at all, since we are not designing for that

💬 furszy commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2413892101)
yeah, that's what I meant.