💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2866204024)
There was a seemingly intermittent CI failure in wallet_basic.py on the commit merging this into master. Tracked as #32456. The following push to master succeeded.

💬 ryanofsky commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2081485161)
Thanks for clarifying. I got the impression that you did think it was useful from "It may be a good idea to actually improve the check here, since it's relied on by `getblocktemplate` in proposal mode and by #31981" (https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2053597806). But I guess you were saying it *could* be useful if it were improved, not that it is useful currently.

Anyway I was mostly asking about this because I was confused. Assuming you are happy with PR in current fo
...

💬 carnhofdaki commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2866215127)
Concept ACK, the patches look fine to me. Compiling 92b05c17b1 right now. Thanks for doing it!

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2081493334)
Good catch! I do think daemon could be ok but I agree node is better. I will update this (and I also want to look into @theStack's suggestion to use subprocess.h, which I was unaware of and could simplify the PR)

💬 laanwj commented on pull request "randomenv: remove some `/proc/` accesses":
(https://github.com/bitcoin/bitcoin/pull/32450#issuecomment-2866302918)
> E.g. simplest implementation: pass a reference to a static flag to every AddFile( and set it to false after a failure. If it's false at start, don't try again.

Alternative using command pattern:
```diff
diff --git a/src/randomenv.cpp b/src/randomenv.cpp
index fbab23afe94315faa65d58d15ee0856875e1e92c..411985c662ea904eeaa983ed87e7597dd1485509 100644
--- a/src/randomenv.cpp
+++ b/src/randomenv.cpp
@@ -94,10 +94,29 @@ void AddSockaddr(CSHA512& hasher, const struct sockaddr *addr)
}
...

💬 hebasto commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2081584114)
This behaviour is MSVC-specific (I've added a note to the PR description). And I cannot reproduce it when running cross-compiled binaries.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081584805)
It can be, and that was why the 1 second timeout check kept failing. I tried a few implementations of the test and it's just impossible to reliably start the test timer in sync with the HTTP server. So sometimes it starts late and you end up with `duration < expected`.

The point of the test is to ensure that the server disconnects idle clients, I think it does that even though the exact time is a little fudged.

💬 hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2866400462)
@laanwj

Are you going to upstream the second commit?

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2866412296)
> Are you going to upstream the second commit?

Yes, but only after cpp-subprocess starts using c++17: https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597743)
Was done on purpose - to retire the type of `Obfuscation` gradually - but I don't mind doing it early either.
Changed in the commit messages as well.

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597874)
Named it as such to minimize diffs - but I also see the logic here - done

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081597972)
Done

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598130)
Was done in the next commit, indeed - localized it, thanks for finding these rebase inconsistencies (likely happened when I changed the order of commits).

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598285)
But I *am* Hungarian :p
In the tests I've been using `key_bytes`, applied it here as well.

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598512)
Valid critique - which is a gateway-suggestion forcing me to admit that we need static extend spans/arrays here :/

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598646)
Hah, indeed, thanks!

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081598779)
Removed, thanks

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2081599213)
Good idea, though it doesn't change the final commit

💬 laanwj commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#issuecomment-2866425607)
Agree with the rationale of this PR, but having 1MB+ binary files in the repo is really meh.

📝 fanquake opened a pull request: "guix: move `*-check.py` scripts under contrib/guix/"
(https://github.com/bitcoin/bitcoin/pull/32458)
These scripts are not meant for general developer usage. They are for use on the release binaries, which have been compiled in an environment that makes various assumptions in regards to c library, compiler options, hardening options, patching etc.

Anyone is free to run these scripts against self-compiled binaries, but this isn't something we want to modifying/generalize the scripts to support.