💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#discussion_r1954429250)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2656487136).

💬 maflcko commented on pull request "fuzz: Use serial task runner to increase fuzz stability":
(https://github.com/bitcoin/bitcoin/pull/31841#issuecomment-2656491879)
CI passed and coverage is as expected:

* This pull: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/faad87189015e559/733f70b21be22925/fuzz.coverage/index.html
* Before this pull: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/6a46be75c43e6489/733f70b21be22925/fuzz.coverage/index.html

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954433827)
I pushed a commit to explain the long poll behaviour. Also added an early return above this point, just for clarity.

💬 ryanofsky commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954434025)
re: https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954308137

> Nit in [a8fedb3](https://github.com/bitcoin/bitcoin/commit/a8fedb36a71ac193fc158284b14d4eb474e5ab62): Why is this needed? It seems like a no-op:

Right it is is a no-op, see #30529 which makes the change you suggested and removes many other misuses of IsArgSet

💬 laanwj commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2656503405)
- Removed the nested loop in `GetAdaptersAddresses` (@sipa)
- Made doc comment for `FromSockAddr` more concise (@davidgumberg)

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2656504648)
I pushed a commit to clarify long poll behavior.

🤔 l0rinc reviewed a pull request: "Benchmark Chainstate::ConnectBlock duration"
(https://github.com/bitcoin/bitcoin/pull/31689#pullrequestreview-2614869602)
Concept ACK, please see my remaining comments:

<details>
<summary>Details</summary>

```patch
Index: src/bench/connectblock.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/bench/connectblock.cpp b/src/bench/connectblock.cpp
--- a/src/bench/connectblock.cpp (revision b3a64758bc09eb60858c23292ba7f1b9a41b9bb9)
+++ b/src/bench/connectblock.cpp (date 17394512
...

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954435541)
can be moved inside the benchmark to make sure previous run doesn't pollute the next one (the inner loop is unrolled and run multiple times so we should reduce dependencies)

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954428945)
Seems unused now

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954439106)
We could avoid copying here as well:
```suggestion
const auto& test_block{CreateTestBlock(test_setup, keys, outputs)};
```

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954436295)
```suggestion
// Doing this in a separate block excludes the validation of its inputs from the benchmark
```

💬 laanwj commented on pull request "net: reduce CAddress usage to CService or CNetAddr":
(https://github.com/bitcoin/bitcoin/pull/31854#issuecomment-2656522537)
Concept ACK

📝 fanquake opened a pull request: "depends: avoid an unset `CMAKE_OBJDUMP`"
(https://github.com/bitcoin/bitcoin/pull/31857)
Similar to #31840, currently our Linux toolchain file contains:
```bash
set(CMAKE_AR "aarch64-linux-gnu-ar")
set(CMAKE_RANLIB "aarch64-linux-gnu-ranlib")
set(CMAKE_STRIP "aarch64-linux-gnu-strip")
set(CMAKE_OBJCOPY "aarch64-linux-gnu-objcopy")
set(CMAKE_OBJDUMP "")
```

`objdump` is currently only used for the macOS cross build, where it's `llvm-objdump`, but we should be consistent in producing a toolchain file that points to actual tools, rather than leaving variables unset.

💬 ryanofsky commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954464935)
> Nit in [a8fedb3](https://github.com/bitcoin/bitcoin/commit/a8fedb36a71ac193fc158284b14d4eb474e5ab62): Would be nice to explain why this is done: Something like: `// GetArgs disregards any debugging categories appearing before -nodebug, so do the same for -debug=0/none`

Could be good to add a reason for this, but the suggested reason is pretty obscure one. I think the general reason for doing this is that later arguments are supposed to take precedence over earlier ones, so when -debug=none
...

💬 l0rinc commented on issue "doc/zmq: Note about endianness does not match reality":
(https://github.com/bitcoin/bitcoin/issues/31856#issuecomment-2656552688)
For reference, @hodlinator fixed a few of these in https://github.com/bitcoin/bitcoin/pull/30526 and @ryanofsky a few more in https://github.com/bitcoin/bitcoin/pull/31596.

💬 0xB10C commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2656566483)
> So if I understand the change correctly, the new (documented/recommended) behaviour is to always first read_arg() which gets the memory location of the data, then read_user{_str}() which then actually performs the copy from user memory to the BPF memory space.

Correct. Here's an example from the `mempool:added` tracepoint test.

- (1) initialize a null-pointer `phash`
- (2) read the first tracepoint argument (a user-space address) into the `phash` pointer
- (3) copy the value behind the
...

💬 hodlinator commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1954481759)
nit: Seems a bit heavy-handed to use exceptions, even if it's following the pattern of code above?
```suggestion
scanning = wallet_info.get("scanning")
if scanning:
assert_greater_than(scanning["duration"], 0)
```
Could also break out `encryped_cli = self.nodes[0].cli("-rpcwallet=encrypted_wallet")` for the entire block now that we are using it for a 4th time, if you re-touch.

💬 maflcko commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954487720)
Thx for confirming. It would be nice to remove it here, so that it doesn't have to be touched (and reviewed) again in another pull request. But it was just a nit and anything is fine.

💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656583674)
> > i had the same problem, even in new test wallets I’ve created with same passpharse, the same problem
>
> Does the problem on a freshly created wallet happen with any passphrase, or only one specific one?

The one specific i described it in my question above sir

👍 l0rinc approved a pull request: "Benchmark Chainstate::ConnectBlock duration"
(https://github.com/bitcoin/bitcoin/pull/31689#pullrequestreview-2614976099)
ACK 7edaf8b64cb2d59ada22042fee62a417e52368b8

I ran the benchmarks and tests locally.
I need a crypto expert to validate them conceptually as well, but I'm fine with the benchmarking code now.

(before merging please fix the typo in the PR description)