💬 dergoegge commented on issue "build: x86 ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2674701021)
So turns out newer versions of afl++ (i.e. newer than the one oss-fuzz is using) will insert `-flto` when `-fcf-protection` is detected. That results in `-flto -fcf-protection -fsanitize=address` and for some reason that combination doesn't work, not sure if it's just not supported or there's a bug but for us a simple workaround is to simply set `-DENABLE_HARDENING=OFF` when fuzzing with afl++ and ASan on x86. The PR that changes this on the afl++ side is: https://github.com/AFLplusplus/AFLplusp
...

💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2674735658)
Rebased and split into three commits for hopefully easier review.

💬 maflcko commented on issue "build: broken CMake *flags output":
(https://github.com/bitcoin/bitcoin/issues/31482#issuecomment-2674744613)
I guess this can be removed from the milestone? Surely, it is ugly, but harmless and the fix seems to be non-trivial and shouldn't be a blocker?

👍 dergoegge approved a pull request: "fuzz: add targets for PCP and NAT-PMP port mapping requests"
(https://github.com/bitcoin/bitcoin/pull/31676#pullrequestreview-2633277793)
utACK c73b59d47f1ec6fff1ad9155181c2285a5ef5cf4

💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1965620497)
Removing?

💬 marcofleon commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1965630562)
Good catch. Took me a bit to see, but yeah this check should be outside of the inner `if` statement.

💬 maflcko commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1965630784)
Previously the assert was exposed, now it is guarded by the same condition, making it dead code that the compiler will remove.

In other words: `{ if (a) assert(a); }` is equivalent to `{ }`.

💬 hebasto commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2674809288)
From the [OP](https://github.com/bitcoin/bitcoin/issues/31047#issue-2570089672):
> It's not clear if using our Coverage build type works with Clang, or not.

There are a few points to take into consideration:
1. Modern LLVM's coverage tool _emulates_ GCC's `gcov` version 4.2.0, which is obviously outdated.
2. Recent versions of the `lcov` tool no longer support GCC/gcov versions older than 9.x. For example:
```
lcov: WARNING: (unsupported) Function begin/end line exclusions not supported with th
...

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2674818845)
My Guix build:
```
aarch64
d94fbbb8bed0a91349c7962d108e0296d6cfef0f4438f9918026fe3e284f5a5a guix-build-568fcdddaec2/output/aarch64-linux-gnu/SHA256SUMS.part
a126b070d9baac09d151e1de17fa21e0f8283f43f0b8bce4c4d53af86f2fdc99 guix-build-568fcdddaec2/output/aarch64-linux-gnu/bitcoin-568fcdddaec2-aarch64-linux-gnu-debug.tar.gz
5a082deacad4c4d970315b6f03bbc9fe7aae2de46cc99709502838b4487d0c57 guix-build-568fcdddaec2/output/aarch64-linux-gnu/bitcoin-568fcdddaec2-aarch64-linux-gnu.tar.gz
696a670e
...

💬 brunoerg commented on pull request "fuzz: split `coinselection` harness":
(https://github.com/bitcoin/bitcoin/pull/31870#discussion_r1965674009)
Ah, good catch! Gonna fix it, thanks.

💬 hebasto commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2674864859)
> So the following workflow works, and is probably what most people do: download with Safari, which automatically extracts the `gz` layer. Then double-click to extract the Tar in Finder. And then start `bitcoind` from the terminal.

I can confirm that this workflow works.

📝 hodlinator opened a pull request: "http: Make server shutdown more robust"
(https://github.com/bitcoin/bitcoin/pull/31929)
- Instead of hanging indefinitely, use timeouts and abort when HTTP connections take unreasonably long to close.
- Add request id with logging to be able to discover which one is stuck.
- Ensure we always free libevent HTTP.
- Improve documentation around libevent behavior.

Should help debug #31894.

💬 fanquake commented on pull request "http: Make server shutdown more robust":
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2674904296)
```bash
Assertion failed: (evb), function WriteReply, file httpserver.cpp, line 682.
Error processing input "/Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets/fuzz_corpora/http_request/83149a7e3abd937c7bc67fa55b9a4fc9338e3d8c"

⚠️ Failure generated from target with exit code 1: ['/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/src/test/fuzz/fuzz', PosixPath('/Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets/fuzz_corpora/http_request')]
```

💬 ryanofsky commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1965736047)
In commit "init: Handle dropped UPnP support more gracefully" (bd4679a608b2cbc206fa60a180b0fc4ce8a39ec4)

I think I might not understanding what this code is trying to do exactly, but it seems overcomplicated and maybe a little off. Like the check for GetSettingPath seems unnecessary, the LogWarning seems vague, the GetPersistentSetting calls not really checking for the right thing as mentioned in an earlier comment. Also the check for isTrue seems overly narrow and the UniValue(true) cast sho
...

🚀 fanquake merged a pull request: "fuzz: add targets for PCP and NAT-PMP port mapping requests"
(https://github.com/bitcoin/bitcoin/pull/31676)

✅ fanquake closed an issue: "lint: Qt exclusions are missing files"
(https://github.com/bitcoin/bitcoin/issues/31801)

🚀 fanquake merged a pull request: "ci: Fix filtering out Qt-generated files from `compile_commands.json`"
(https://github.com/bitcoin/bitcoin/pull/31928)

🚀 fanquake merged a pull request: "contrib: update `utxo_to_sqlite` tool documentation and comment"
(https://github.com/bitcoin/bitcoin/pull/31925)

🚀 fanquake merged a pull request: "fuzz: add basic TxOrphanage::EraseForBlock cov"
(https://github.com/bitcoin/bitcoin/pull/31918)

💬 ryanofsky commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1965781753)
In commit "init: Handle dropped UPnP support more gracefully" (bd4679a608b2cbc206fa60a180b0fc4ce8a39ec4)

I think it would be better to move the settings.json update code before this code doing "if upnp then softset natpmp". That way soft-setting natpmp only happens if -upnp is enabled in `bitcoin.conf` or on the command line, not if it is enabled in settings.json. So the separate cases will be handled independently and not interacting in a more complicated way.