📝 laanwj opened a pull request: "net: Prevent accidental circuit sharing when using Tor stream isolation"
(https://github.com/bitcoin/bitcoin/pull/32176)
Add a class TorsStreamIsolationCredentialsGenerator that generates
unique credentials based on a randomly generated session prefix
and an atomic counter.

This makes sure that different launches of the application won't share
the same credentials, and thus circuits, even in edge cases.

Example with `-debug=proxy`:
```
2025-03-31T15:51:20Z [proxy] SOCKS5 sending proxy authentication d91418ab1b2fc8d7b672a6eb3ac96f8f-0:d91418ab1b2fc8d7b672a6eb3ac96f8f-0
2025-03-31T15:51:21Z [proxy] SOCKS
...

💬 darosior commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#issuecomment-2766799625)
Concept ACK.

I don't buy that the discontinuity would cause any more confusion than making harmless transactions non-standard. The upgrade hook argument made sense back when this PR was closed, but i don't think it holds anymore now that no proposal includes invalidating <64 bytes transactions.

> OTOH, it might be plausible to just not worry about small-tx's at all, in which case the check could just be removed entirely, avoiding any discontinuities...

This goes completely against your
...

💬 glozow commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2021216973)
concept ack introducing this constant. I think it would be best to have it in a separate commit and replace the magic number in policy.cpp with this

💬 glozow commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2021218605)
Should use `CURRENT_VERSION` instead of magic number

💬 glozow commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2021201880)
Yes, this seems to be redundant with `TX_MAX_STANDARD_VERSION`. It also appears to be unused, did you maybe forget to delete it?

💬 glozow commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r2021361765)
This is needed as the arg is non-string type. My understanding is that @luke-jr is concept nacking the PR. I don't really understand the comments about the positionalness of this arg?

📝 instagibbs opened a pull request: "TxGraph: Increase fuzz coverage"
(https://github.com/bitcoin/bitcoin/pull/32177)
Was looking at my local coverage report, and noticed a few spots that will not or cannot be hit.

CountDistinctClusters, GetAncestorsUnion, and GetDescendantsUnion accept nullptrs, but the test harness never employs them. Disallow them.

We never call PullIn whenever there isn't staging, so just enforce that invariant via assertion.

Remaining places that are not covered:

1) Relinearize: Currently we seem to always start with a cold (not known to be optimal) cluster, and after one attem
...

👍 instagibbs approved a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2729193453)
ACK d8e4b92fb4aea16e18285d25c4d5bba3b3b51c98

non-blocking comments

`git range-diff master 0630995fee22990402771547be1480b8706c76ce d8e4b92fb4aea16e18285d25c4d5bba3b3b51c98`

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2021185973)
Yeah I think no comment at all is better than as-is, but maybe we can just point exactly to what case this is ignoring as safe?

"Sort by feerate only, since violating topological constraints within same-feerate chunks won't affect diagram comparisons."

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2021048633)
hindsight nit: `chunking` could be `const` to make it clear loop end condition isn't being modified

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2020997878)
```Suggestion
/** Get feerate diagrams for both main and staging respectively (which must both exist and not be
```
or be even more pedantic about ordering

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2021186944)
not worth the nit, mark as resolved please

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2020995996)
Actually we need to define exactly what a feerate diagram is here given it could mean a couple things. i.e., it's not the aggregated form, but a series of monotonically decreasing in feerate chunks

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2021150749)
f01ce7f2c9d50a95bb7c694028525c8cc3112a56
```Suggestion
Assume(m_cur_cluster);
m_cur_cluster->GetClusterRefs(*m_graph, ret->first, start_pos);
```

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2021344681)
both cases could use fuzz coverage :pray:

💬 ajtowns commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#issuecomment-2766949251)
> > OTOH, it might be plausible to just not worry about small-tx's at all, in which case the check could just be removed entirely, avoiding any discontinuities...
>
> This goes completely against your earlier argument of keeping our options open with regard to upgrade hooks! There is a current proposal to make those transactions invalid, so making them standard now seems unnecessarily risky.

If there is a need to worry about small transactions, then the conservative thing to do in consensu
...

💬 stratospher commented on issue "validation: `CheckBlockIndex` crashes during block reconsideration":
(https://github.com/bitcoin/bitcoin/issues/32173#issuecomment-2767030982)
I came across this error too when playing with @mzumsande's fuzzer in https://github.com/bitcoin/bitcoin/pull/31533!

since `reconsiderblock` doesn't traverse the whole tree of block headers and only ancestors/descendants, we could have failed blocks which don't descend from invalid blocks. (there's a picture below showing a similar situation.)

2 possible solutions:
1. modify [the check](https://github.com/bitcoin/bitcoin/blob/15717f0ef3960969ee550a4a41741987b86684dc/src/validation.cpp#L5400)
...

💬 sipa commented on pull request "TxGraph: Increase fuzz coverage":
(https://github.com/bitcoin/bitcoin/pull/32177#issuecomment-2767038988)
utACK a40bd374aaf8e10116fa664fa7b480d85ee2fe59

💬 mzumsande commented on issue "validation: `CheckBlockIndex` crashes during block reconsideration":
(https://github.com/bitcoin/bitcoin/issues/32173#issuecomment-2767097609)
A third possible fix: change `pindexFirstInvalid` so that it is set both on `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD`:
``` diff
diff --git a/src/validation.cpp b/src/validation.cpp
index fedcb9ca57..4fea14b2a5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5323,7 +5323,7 @@ void ChainstateManager::CheckBlockIndex()

while (pindex != nullptr) {
nNodes++;
- if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex
...

💬 hodlinator commented on pull request "torcontrol: Define tor reply code as const to improve our maintainability":
(https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2021623227)
> Seems the benefit of enum class would be to make sure they're grouped by name `TorReply::OK` `TorReply::UNRECOGNIZED`?

Yes, but then one would have to do `if (foo->bar == static_cast<int>(TorReply::OK))` which becomes a bit verbose.

🤔 ryanofsky reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2730181907)
Rebased 625bd576215e82d430998bfa68501db7ba03c3b2 -> d31c53455f23788eb09f934dad0eaadec2ee1b92 ([`pr/subtree.26`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.26) -> [`pr/subtree.27`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.27), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.26-rebase..pr/subtree.27)) incorporating https://github.com/bitcoin-core/libmultiprocess/pull/168 and making other suggested changes