💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2429694223)
good call ill set a `static constexpr` for all the times we do this in the module, overriding https://github.com/bitcoin/bitcoin/pull/32747/files#r2420892943

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2421943022)
I'll add coverage for as many as I can. But I think all I can do from this level is try to bind to an address with an invalid family (i.e. "NET_ONION").

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2421300798)
This is a reasonable note but I think it's out of scope for this PR, since `GetSockAddr()` is used so many other places in the code. Wrapping `sockaddr_storage` for C++ style could also be applied to `Accept()`, `Connect()`, `Bind()` and probably a few others. I'll open a new PR for that cleanup, similar to https://github.com/bitcoin/bitcoin/pull/33378

💬 waketraindev commented on pull request "rpc, test: fix "internal" label check in importdescriptors and extend test":
(https://github.com/bitcoin/bitcoin/pull/33614#issuecomment-3402968735)
Fixes #32376, same underlying issue as #31514
Happy to defer to #31514 if it's preferred.

💬 sipa commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2430034830)
Fixed (again).

💬 sipa commented on pull request "TxGraph: change m_excluded_clusters":
(https://github.com/bitcoin/bitcoin/pull/33469#issuecomment-3403060956)
ACK 9b43428c96872f0fbbbab4c066c6010fc18c6cc4

🤔 stratospher reviewed a pull request: "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation"
(https://github.com/bitcoin/bitcoin/pull/33604#pullrequestreview-3336930055)
ACK 9f946a7.

nice! I think this aligns with the original intention to restrict the peer only when background sync isn't over in https://github.com/bitcoin/bitcoin/pull/29519#discussion_r1635456871 where this code was introduced.

looks like we might incorrectly set `pindexLastCommonBlock` in [L1401](https://github.com/bitcoin/bitcoin/blob/9f946a79ca894741547c5892f9dc60d023ae1508/src/net_processing.cpp#L1401) to the previous best known block tip again in this scenario. but it's not a problem
...

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2430088556)
Yes I think so!

⚠️ glozow opened an issue: "Minor Release 29.3"
(https://github.com/bitcoin/bitcoin/issues/33628)
Backports
- https://github.com/bitcoin/bitcoin/pull/33611

<!--
RC
- [] final changes
- [] tag
- [] upload binaries
- [] announcements
-->

See [release process docs](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md) for more details. This issue can be used to track status updates. Please feel free to suggest backports in the comments.

💬 brunoerg commented on pull request "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`":
(https://github.com/bitcoin/bitcoin/pull/33624#discussion_r2430105092)
Done.

💬 brunoerg commented on pull request "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`":
(https://github.com/bitcoin/bitcoin/pull/33624#discussion_r2430105307)
Done.

💬 brunoerg commented on pull request "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`":
(https://github.com/bitcoin/bitcoin/pull/33624#issuecomment-3403144907)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/33624#discussion_r2429811753 and https://github.com/bitcoin/bitcoin/pull/33624#discussion_r2429816763.

👍 instagibbs approved a pull request: "test: P2SH sig ops are only counted with `SCRIPT_VERIFY_P2SH`"
(https://github.com/bitcoin/bitcoin/pull/33624#pullrequestreview-3336988018)
ACK 3a10d700bc1889b3690097efc935c5a4ba5966bb

💬 brunoerg commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2430117604)
e6c439dcb23e1a0d03261734fd89412a15b1932a: I don't this it needs this "Full Description:". It seems obvious that is the description of the algorithm.

```suggestion
This algorithm works by selecting inputs randomly until the target amount is
```

💬 brunoerg commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2430157941)
Aren't there already documentation about the parameters in `coinselection.h`? If so, we should just add the description there instead.

💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3403249590)
> Is the working assumption that the errors users are reporting in #26391 are due to disk corruption causing exactly the serialized `nStatus::BLOCK_FAILED_{VALID,CHILD}` bit to be flipped, causing the block to be loaded as invalid instead of valid, without causing any other corruption that would likely crash the application? Because while that wouldn't be impossible, it seems like an incredibly unlikely event to me?

No, that would indeed be unlikely. The assumption is that some kind of corrup
...

🤔 hodlinator reviewed a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-3337135597)
Concept ACK 492be23a1873d7165e6cc75d1282f4feb63f87b9

💬 hodlinator commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2430217115)
Tried cherrypicking

<details><summary>minimal diff to change m_nodes_mutex mutex type and ForNode usage</summary>

```diff
diff --git a/src/net.h b/src/net.h
index d806b42a1e..f997bbe820 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1519,7 +1519,7 @@ private:
mutable Mutex m_added_nodes_mutex;
std::vector<CNode*> m_nodes GUARDED_BY(m_nodes_mutex);
std::list<CNode*> m_nodes_disconnected;
- mutable RecursiveMutex m_nodes_mutex;
+ mutable Mutex m_nodes_mutex;
std::atomic<No
...

✅ sdaftuar closed a pull request: "Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676)

📝 sdaftuar opened a pull request: "Cluster mempool "
(https://github.com/bitcoin/bitcoin/pull/33629)
[Reopening #28676 here as a new PR, because GitHub is slow to load the page making it hard to scroll through and see comments. Also, that PR was originally opened with a prototype implementation which has changed significantly with the introduction of `TxGraph`.]

This is an implementation of the [cluster mempool proposal](https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393).

This branch implements the following observable behavior changes:

- Maintains a partit
...