✅ fanquake closed an issue: "CI: Improve documentation around replicating CI locally"
(https://github.com/bitcoin/bitcoin/issues/31199)

💬 roconnor-blockstream commented on issue "Standardness policy rules for legacy Multisig script is incoherent":
(https://github.com/bitcoin/bitcoin/issues/33882#issuecomment-3559368666)
I agree. I think my next step would be to PR a change to `AreInputsStandard` to bring P2SH and legacy script policy into alignment.

I'm not quite sure when I'll get to this, so if someone else wants to take a crack at it, I wouldn't be surprised if they can do it better / sooner than me.

👍 l0rinc approved a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3487360414)
I love this change, it's really well organized, the commit messages explain each change, there's a lot of context - maybe too much, spread around in multiple PRs.

Concept ACK from me, managed to review until a4fb628d83a344bd866715cee63d7dd9ff03dfff - and peeked into the rest.

My understanding is that this isn't compatible with package relay - what about and cluster mempool, how much will we have to change after it's merged?

Given that this has been in review for a while - I wonder how much we
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546177051)
nit:

```suggestion
active_connections += node->IsPrivateBroadcastConn();
```

also note that this isn't exercised currently in the tests (but after this change, it would):
<img width="644" height="185" alt="image" src="https://github.com/user-attachments/assets/81bbe700-5db1-4626-b0fd-ac16905c4593" />

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2545675641)
nit: to avoid [slicing](https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&branch=29415-07650a49f97801701e22af56e6dcba519082eb4b&id=aureleoules_bitcoin&open=AZqcgyl6QjpDlMIZ3R_B&tab=code):
```suggestion
your_addr = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? CService{addr} : CService{};
```

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546085333)
is this used anywhere? https://corecheck.dev/bitcoin/bitcoin/pulls/29415 claims it's uncovered new code

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2545798247)
I started commenting here, which lead me to read the related comments about this line - there's a lot of history here.
I'm still not sure I understand why this is an identifiable constant (which limits the anonymity set to private broadcasters) and not a randomized set of our currently available peer versions or other possible values (as also suggested in https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1517904089, which would also cast doubt on the origin of non-private transactions,
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2545669887)
nit: as https://corecheck.dev/bitcoin/bitcoin/pulls/29415 also [mentioned](https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&branch=29415-07650a49f97801701e22af56e6dcba519082eb4b&id=aureleoules_bitcoin&open=AZqcgyrEQjpDlMIZ3R_G):
> Define operator<=> and remove operators <, <=, >, >= for operand types "Priority" and "Priority".

maybe something like:
```patch
diff --git a/src/private_broadcast.cpp b/src/private_broadcast.cpp
index 487b071ac9..5c23bfa830 100644
--- a/src/
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546155699)
Wasn't sure whether the range of 0-5 minutes implied that the max was 14 minutes (i.e. whether the millisecond type was taken into consideration), so I ran it a few times and the plots indicate it's correct:

<img width="1008" height="618" alt="Image" src="https://github.com/user-attachments/assets/526e142c-0b77-41ab-ab3d-306a132cf7e1" />

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546260152)
is my understanding correct that malleated transactions can at worst leave the tx temporarily in the private broadcast queue?

nit: typo in [commit message](https://github.com/bitcoin/bitcoin/pull/29415/commits/abf841de0c504a512c559e0a056f5ca33831224f): `wxtid`

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546287626)
is my understanding correct that:
* there's no retry count limit or final timeout for retries?
* restarting the node won't continue the retries?

Can we add some unit tests for the code so that it's not only exercisably by python? Would help with debugging to understand the feature in isolation. And the multi-threaded code is a bit concerning, could we fuzz it to make sure there are no races there that are hard to reason about otherwise?
And based on the code coverage it seems to me some of
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2546348544)
nit: maybe we can separate the happy path from giving up via something like:
```suggestion
if (auto it_node{m_by_nodeid.find(nodeid)}; it_node != m_by_nodeid.end()) {
if (auto it_tx{m_by_txid.find(it_node->second)}; it_tx != m_by_txid.end()) {
return it_tx->second.tx;
}
}
return std::nullopt;
```

💬 l0rinc commented on pull request "clang-format: Set Bitcoin Core IncludeCategories":
(https://github.com/bitcoin/bitcoin/pull/33917#discussion_r2547132580)
this might as well be false, lol

💬 l0rinc commented on pull request "clang-format: Set Bitcoin Core IncludeCategories":
(https://github.com/bitcoin/bitcoin/pull/33917#discussion_r2547133633)
for simplicit, this can also be `false`

💬 l0rinc commented on pull request "clang-format: Set Bitcoin Core IncludeCategories":
(https://github.com/bitcoin/bitcoin/pull/33917#discussion_r2547130625)
nice!

How did you come up with the categories? Have you reformatted the whole codebase - or it's just experience? :)

💬 l0rinc commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2547146360)
Good idea. Should we do it in a follow-up instead?

💬 l0rinc commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2547148360)
I have tried explaining it in the commit message, can you please point out where more context is necessarry?

💬 davidgumberg commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2547165872)
I'm not sure who else is using this logging message, but I don't think it's very helpful to print 5, in all of my branches investigating compact block reconstruction I remove this `if` statement entirely

I think we should print all of the missing transactions every time, but can definitely be discussed in a separate issue/pr.

💬 jonatack commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-3559469161)
CLI calls can be made to a node running a different, older version. If a researcher, data gatherer, developer, or node operator is calling this on a pre-getaddrmaninfo node, then the call should still work. As commented above, colleagues here requested that -netinfo continue to work in such cases, and we patched it. There doesn't seem to be any reason not to do the same here, rather than choosing to break it.

💬 ryanofsky commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3559484781)
My takeaway from IRC meeting ([log](https://achow101.com/ircmeetings/2025/bitcoin-core-dev.2025-11-20_16_02.log.html)) is that Matt broadly wants core to be more opinionated and make more decisions on behalf of clients, while Sjors is trying to provide a more unopinionated interface. I think there is actually not much tension between these two things because we could implement everything Matt wants without changing the interface. The IRC discussion just looked to me like a debate about which opt
...