💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2111687366)
Ok, fair enough. It may be fine, and should be trivial to revert, if it doesn't work as expected.

However, I worry a bit about the output not being copy-paste-able for developers to apply to their source code.

Not sure if this is a valid concern, but an alternative approach would be to just call `git diff` below on the `TARGETS_WITH_FORCED_IWYU` folders (not targets). Then exit, when the diff is not empty?

This should also address the issue of having to run twice.

The only downside I
...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2111679951)
nit: no need to disable shellcheck here via `bash -c`?

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2111680762)
nit: Could inline the `TARGETS_WITH_FORCED_IWYU` to avoid the `bash -c` disable of shellcheck?

🤔 rkrux reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2874402429)
ACK fdabfc7b01ec2cff5f2356e1b13b5537d30dbaf8

Thanks for incorporating the previously suggested small changes.

💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2111461649)
```python
or c == "'"
```

I don't think checking for apostrophe is really required specifically in this case because as I checked in the code that only the hardened derive types are used when the SPKMs are setup & that's what this test is using.
https://github.com/bitcoin/bitcoin/blob/88b22acc3d6faeb2c0860695783a2a082d4c8098/src/wallet/walletutil.cpp#L58-L75

And the `ToNormalizedString` doesn't use the compact string version as well.
https://github.com/bitcoin/bitcoin/blob/88b22acc3d6
...

💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2111471325)
I wanted to see how the case would be handled by `CHECK_NONFATAL` when for some buggy reason the call here fails. I simulated a scenario and found that the error is propagated to the RPC result - good.

```bash
➜ bitcoin git:(getaddrinfo-normalized-parent) ✗ bitcoinclireg listunspent
error code: -1
error message:
Internal bug detected: false
wallet/rpc/util.cpp:115 (PushParentDescriptors)
Bitcoin Core v29.99.0-a759a22d59e8-dirty
Please report this issue here: https://github.com/bitcoin
...

💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2916237433)
> Thanks for the suggested fix @hebasto! Bumping the guix time machine to

I don't think we need to bump anything here (putting aside the fact that the suggested change breaks the riscv & windows builds, and will make many other changes to the release env). We can just apply a one line patch to LIEF. i.e:
```diff
Partially revert f23ced2f4ffc170d0a6f40ff4a1bee575e3447cf

Restore compat with python-scikit-build-core 0.10.x
Can be dropped when using python-scikit-build-core 0.11.x

--- a
...

💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2916430210)
> ```diff
> Restore compat with python-scikit-build-core 0.10.x
> Can be dropped when using python-scikit-build-core 0.11.x
> ```

It seems should be:
```
Restore compat with python-scikit-build-core 0.9.x
Can be dropped when using python-scikit-build-core 0.10.x
```

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2916452571)
Rebased 65fe5d03e7a2d0d00d7d37bd426fd6532fff3c06 -> 1417e0b3b1b03dd014a3459c10a5ae7ab0c3687f ([kernelApi_38](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_38) -> [kernelApi_39](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_39), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_38..kernelApi_39))

* Fixed conflict with #32528

💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2916512903)
ACK https://github.com/bitcoin/bitcoin/pull/31553/commits/e1f501cab224608004e3a1eee7a48eec3cea25fc

I appreciate the additional coverage added; I know it's not particularly blackbox but should catch the most basic regressions going forward.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2112031978)
Thanks! Done.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2112032547)
Thanks! Done.

💬 theuni commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#discussion_r2112057623)
> So, my assumption here is that when the "unlock capability" that ReverseLock takes is released generically, the thread safety analyzer can't be sure whether this was an exclusive capability or a shared one, consequently it is ambiguous whether the resource is locked again, because ReverseLock may have been one of many that took "unlock capability" on the lock, is that what the problem is? If release_generic can't disambiguate these situations, what is it even useful for annotating?

My best
...

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2916658444)
I'm interested in seeing benchmarks of this on various systems, especially high-end/modern ones (as their performance is likely most predictive of what future hardware will be like).

```
./build_dev_mode/bin/bench_bitcoin --filter="Linearize.*Example.*" -min-time=10000
```

Here are my own:

#### AMD Ryzen 9 5950X 16-Core Processor

| ns/cost | cost/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:-------
...

📝 mzumsande opened a pull request: "test: fix sync function in rpc_psbt.py"
(https://github.com/bitcoin/bitcoin/pull/32630)
Even though the block is created on `node2`, the sync is only between `node1` and `node0`. Accordingly the test fails if I put a sleep in `msg_type == NetMsgType::HEADERS` processing: In this, case `node1` and `node0` do not hear about the new block, the sync still passes because they are in sync with each other, and later on in the `test_input_confs_control` subtest, `node1` would generate a forked block instead of building on the previous one, leading to test failure.

Haven't seen this in
...

💬 maflcko commented on pull request "test: fix sync function in rpc_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/32630#issuecomment-2916784818)
lgtm ACK 4df4df45d7bc2e8be99325d40cda936aab87c083

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2111949140)
In "ipc: Add bitcoin-mine test program" 6841bae335d109e0207aed002635cd12e609f9a8

When their is no bitcoin-node process we don't spawn a new one but exit instead.
I attempt to call `spawnProcess` and pass `bitcoin-node` as executable.

The client crashes

```
2025-05-28T15:06:39Z Default data directory /Users/abubakarismail/Library/Application Support/Bitcoin
2025-05-28T15:06:39Z Using data directory /Users/abubakarismail/Library/Application Support/Bitcoin
2025-05-28T15:06:39Z Config
...

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2111970868)
In "ipc: Add bitcoin-mine test program" 6841bae335d109e0207aed002635cd12e609f9a8

The valid address probably shouldn’t be hardcoded here. It would be better to have a function that returns all valid addresses, with a default constant for unix.

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2111981130)
In "ipc: Add bitcoin-mine test program" https://github.com/bitcoin/bitcoin/commit/6841bae335d109e0207aed002635cd12e609f9a8

Maybe specify the default datadir

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2112177316)
In "ipc: Add bitcoin-mine test program" https://github.com/bitcoin/bitcoin/commit/6841bae335d109e0207aed002635cd12e609f9a8

Should validate the address first?