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

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2112219834)
I attempted it in https://github.com/ismaelsadeeq/bitcoin/commit/c714339f6bdce1be68ce127be98dffdae56dbaa4

With an added test to the `Echo` interface.

📝 marcofleon opened a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631)
Part of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).

This PR changes the GenTxid class to a variant, which holds both Txids and Wtxids. This provides compile-time type safety and eliminates the manual type check (bool m_is_wtxid). Variables that can be either a Txid or a Wtxid are now using the new GenTxid variant, instead of uint256.

💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#issuecomment-2916823179)
In `txrequest` the `ByPeer` and `ByTxHash` indexes still sort announcements by the underlying uint256 txhash only, as opposed to using the GenTxid comparators which sort by type and then hash. The patch below can be used to make sure that there are no accidental switches from uint256 comparisons to GenTxid comparisons.
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 011e3cd928..1a47a39dfd 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -206,6
...

💬 theuni commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-2916839229)
Pushed a small additional commit that removes a now-unnecessary template instantiation. Just a little janitorial work, but it will make some future changes more obviously correct.