✅ stringintech closed a pull request: "kernel: Rename in-memory DB option setters and simplify API"
(https://github.com/bitcoin/bitcoin/pull/33877)

💬 stringintech commented on pull request "kernel: Rename in-memory DB option setters and simplify API":
(https://github.com/bitcoin/bitcoin/pull/33877#issuecomment-3580764433)
Fair points! Thank you for the feedback.

Closing as this is not as helpful as I initially thought.

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#issuecomment-3580784708)
Closing - these commits are all merged into #32061 including all the great review feedback <3

✅ pinheadmz closed a pull request: "Introduce SockMan ("lite"): low-level socket handling for HTTP"
(https://github.com/bitcoin/bitcoin/pull/32747)

👋 pinheadmz's pull request is ready for review: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061)

💬 maflcko commented on issue "RFC: Add versioning to the kernel header":
(https://github.com/bitcoin/bitcoin/issues/33911#issuecomment-3580786554)
It is probably fine for the first few releases to just treat the Bitcoin Core `FormatFullVersion()` as the kernel version. It is clear that that version does not follow the semantic versioning of the kernel API, but if kernel users care about breaking changes, they will be listed in the release notes anyway.

💬 pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3580788520)
Ok this PR is back up for review. It is divorced from SockMan, and the I/O loop is now implemented inside `HTTPServer` itself.

💬 stickies-v commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2564545589)
Good catch @l0rinc, I didn't test this in my review, and you're right, it doesn't work (`AttributeError: 'Namespace' object has no attribute 'out_sdkt_path'`) on master nor in the current HEAD. I also agree with removing `nargs=1`:

<details>
<summary>git diff on 266a2bf40e</summary>

```diff
diff --git a/contrib/macdeploy/gen-sdk.py b/contrib/macdeploy/gen-sdk.py
index e9221cf60d..bfac1542a2 100755
--- a/contrib/macdeploy/gen-sdk.py
+++ b/contrib/macdeploy/gen-sdk.py
@@ -21,7 +21,7 @@
...

👍 theStack approved a pull request: "test: Fix reorg patterns in tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587#pullrequestreview-3510402810)
re-ACK 70d9e8f0a15d07a27ae37befb5c1bce71c98d8de

💬 fanquake commented on pull request "depends: latest config.guess & config.sub":
(https://github.com/bitcoin/bitcoin/pull/33945#issuecomment-3580861967)
Guix Build (aarch64):
```bash
50d2ead905b87b4064fd66a0c0a2dc88a7ada65e2224f1ff992bc7ec685e493d guix-build-3e4355314b1a/output/aarch64-linux-gnu/SHA256SUMS.part
923b405f4eb8cb7158f2b4aef0918e48e7e57ef097871d0702dbbd5286013d32 guix-build-3e4355314b1a/output/aarch64-linux-gnu/bitcoin-3e4355314b1a-aarch64-linux-gnu-debug.tar.gz
10317ea4cbaf9b6911592d0d4636d5959bf31f8ad0210cfe8decc4a5ebaa99fb guix-build-3e4355314b1a/output/aarch64-linux-gnu/bitcoin-3e4355314b1a-aarch64-linux-gnu.tar.gz
70bcea
...

👍 rkrux approved a pull request: "wallet: Expand MuSig test coverage and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33636#pullrequestreview-3510456391)
lgtm ACK 217dbbb

Thanks for accepting all the suggestions.

🚀 fanquake merged a pull request: "depends: latest config.guess & config.sub"
(https://github.com/bitcoin/bitcoin/pull/33945)

💬 rkrux commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#issuecomment-3580889312)
In PR description:

```diff
- primarily adds test coverage for some of the most prominent failure cases in the first commit.
+ primarily adds test coverage for some of the most prominent failure cases in the last commit.
```

⚠️ maflcko opened an issue: "node0 stderr bitcoind: validation.cpp:5392: void ChainstateManager::CheckBlockIndex() const: Assertion `!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))' failed."
(https://github.com/bitcoin/bitcoin/issues/33948)
Just ran into this crash in the nightly CI, which was using llvm `22~++20251125085442+ed95c4d6ecf0-1~exp1~20251125085501.1311`. It may be a compiler error, but it is odd that only one test was failing:

https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19692916493/job/56412271437#step:10:6430

```
node0 stderr bitcoind: validation.cpp:5392: void ChainstateManager::CheckBlockIndex() const: Assertion `!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))' failed.
```

...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2564638844)
> The first two are just optimizations for logarithmic operations

Just a minor correction, not relevant with the recent changes, but anyway - the first container, "by txid" is where the transactions are stored. The 2nd and 3rd are the indexes. "by priority" is (was) logarithmic and "by node" was `O(1)`.

In the latest push I removed those.

Also, "priority" is something that changes when the transaction is sent more times. So, I went in the direction of storing how many times and when a t
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2564640474)
"Resolving" since this code was removed.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2564641818)
"Resolving" since this code was removed.

👍 stickies-v approved a pull request: "Change Parse descriptor argument to string_view"
(https://github.com/bitcoin/bitcoin/pull/33914#pullrequestreview-3510482245)
ACK c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3

Thanks for improving this. I switched to taking a span in b3bf18f0bac0ffe18206ee20642e11264ba0c99d to minimize code changes, but didn't think of the string literal case. For a string parsing function, passing string literals should just work out of the box, so this approach is definitely better.

💬 stickies-v commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2564635705)
nit: I don't think `sp` is a particularly helpful variable name, would prefer `descriptor_span`

💬 stickies-v commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2564629268)
nit: doesn't make a functional difference because it's the end of the test case, but this should be `CHECK` since nothing further depends on `desc`'s non-emptiness

```suggestion
BOOST_CHECK_MESSAGE(!descs.empty(), err);
```

💬 maflcko commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#issuecomment-3580916853)
rebased for a one-line conflict and a one-line silent conflict. Should be trivial to re-review via git range-diff