💬 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.

💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2866437307)
> But moving both to a guix-specific path would be fine with me.

See #32458.

💬 l0rinc commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#issuecomment-2866447793)
Agree - do you have a better idea?

🤔 maflcko reviewed a pull request: "bench: replace benchmark block with more representative one (413567 → 784588)"
(https://github.com/bitcoin/bitcoin/pull/32457#pullrequestreview-2827710648)
Is there a benchmark that needs this? If yes, going for synthetic, but representative (and easily adjustable) data may be a better choice for that benchmark.

💬 maflcko commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2081323234)
Instead of a block, this could just be random bytes from a fast random context?

🤔 TheCharlatan reviewed a pull request: "build: simplify *ifaddr handling"
(https://github.com/bitcoin/bitcoin/pull/32446#pullrequestreview-2828221038)
lgtm, waiting for guix build.

🚀 fanquake merged a pull request: "depends: Avoid using helper variables in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31360)

💬 Sjors commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2081627012)
> since it's relied on by getblocktemplate in proposal mode

That was a mistake on my end, because `checkBlock` calls `connectBlock` which does the full check.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081628843)
Great catch! I found [this:](https://docs.python.org/3/library/socket.html#socket.timeout)

> `exception socket.timeout`
> A deprecated alias of [TimeoutError](https://docs.python.org/3/library/exceptions.html#TimeoutError).
> Changed in version 3.10: This class was made an alias of [TimeoutError](https://docs.python.org/3/library/exceptions.html#TimeoutError).

Since Python 3.10 is minimum version required in dependencies.md (and the test passes!) I think it's ok to leave as-is

💬 maflcko commented on pull request "refactor: Removals after bdb removal":
(https://github.com/bitcoin/bitcoin/pull/32438#discussion_r2081633399)
I guess it could make sense to use a previous release where bdb wasn't yet deprecated, but not sure if this is worth it, and it could be a follow-up. Removed this commit for now, thanks.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#issuecomment-2866480355)
> Maybe the three commits could be squashed?

Done! thanks for reviewing

👍 theStack approved a pull request: "psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows"
(https://github.com/bitcoin/bitcoin/pull/32419#pullrequestreview-2828263249)
ACK d31158d3646f3c7e4832b9ca50f6ffe02800ff4c

The introduced comments match my understanding of the deserialization of PSBT types and will hopefully help reviewing future code introduced/touched in this area. It's not ideal that the same comments are copied at three places, but don't have a better idea how to avoid this (still better to have duplicated documentation than no documentation).

💬 TheCharlatan commented on pull request "guix: move `*-check.py` scripts under contrib/guix/":
(https://github.com/bitcoin/bitcoin/pull/32458#issuecomment-2866517322)
Concept ACK

💬 Sjors commented on pull request "refactor: Removals after bdb removal":
(https://github.com/bitcoin/bitcoin/pull/32438#issuecomment-2866544280)
ACK fa061bfcdb0caea240fd15bcc309e7847132a4ff if CI is also happy

I didn't attempt a vcpkg build (fa5f3e62c8801cca80997cfb046c13983e0876e7), but I assume the Windows CI does that?

I didn't check for _additional_ things that could be removed. Though there is bunch of watch-only related code in the GUI that can be dropped.

🤔 vasild reviewed a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2828006168)
Almost ACK 3acfc071c3445e943069b2778bbc5c74f702ef36

Only unsure about `The configured rpcuser or rpcpassword cannot contain a ":"`, see comment below.

💬 vasild commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2081681184)
This looks like a new restriction. Could it introduce a breakage for users that have `-rpcpassword=contains:colon`?

It is sub-optimal to join/craft the string "user:pass" and to split/parse it a few lines later. Better store the user and pass in separate variables, like this:

<details>
<summary>[patch] use separate variables for user and pass</summary>

```diff
diff --git i/src/httprpc.cpp w/src/httprpc.cpp
index ed5a0253c6..493f1b0320 100644
--- i/src/httprpc.cpp
+++ w/src/httprpc.
...