💬 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

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021575880)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2020675936

I don't actually understand what those ranges mean so I've just been using the typical convention listing years (https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1999444438)

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021578441)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021118543

Thanks dropped this line now

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021569778)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2020674237

Thanks, switched to cmake_dependent_option

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021584777)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2020678945

> Why does building the `mpgen` tool require `core_interface`?

I don't think it does require it directly because `mpgen` depends on the `multiprocess` target which already depends on `core_interface`.

But everywhere else we seem to add `core_interface` as a dependency of executable targets whether necessary or not, and this seems like a good thing to ensure flags are applied consistently without needing to know abo
...

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021576733)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2020697414

I was surprised to see bitcoin build is not calling `enable_testing()` when `BUILD_TESTS` is off, so it seems like the best thing to do here is to to set [`BUILD_TESTING`](https://cmake.org/cmake/help/latest/variable/BUILD_TESTING.html) to match `BUILD_TESTS` since they are doing the same thing. Updated push does this and replaces the comment.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2021594145)
re: https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2020683263

Thanks, updated in latest push to depend on `BUILD_TESTS` setting as suggested. I also updated `BUILD_TESTS` documentation to reflect this.

💬 ChayDragan commented on issue "estimateSmartFee error: "Insufficient data or no feerate found"":
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2767164834)
@pinheadmz @willcl-ark -

This issue seems to persist on both testnet3 and testnet4, is there other fixes coming or recommended outside of manually deleting files over and over again?

We expected that testnet4 would resolve this issue and bring a more stable environment but unfortunately has continued to provide an instable testing environment. Any help would be greatly appreciated.

⚠️ ChayDragan opened an issue: "estimateSmartFee error: "Insufficient data or no feerate found"
(https://github.com/bitcoin/bitcoin/issues/32178)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

To estimate fees we use estimateSmartFee and were constantly running into the following error: estimateSmartFee error: "Insufficient data or no feerate found"

As mentioned in other github issues, we could manually delete fee_estimate.dat and mempool.dat from the node. Which isnt scalable, so we moved to testnet4 with the understanding it would provide a more stable testing environment.


...

💬 janb84 commented on pull request "fuzz: Make partially_downloaded_block more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32158#issuecomment-2767254430)
The 128 run fails on my machine: `**fuzz failed: Too many open files (os error 24)**`
```shell
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build/ $PWD/qa-assets/fuzz_corpora/ partially_downloaded_block 128
```

The 32 run works. I'm on a M1 MAX, in a nix-shell clang 19.1.7, cargo 1.85.0

🚀 ryanofsky merged a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134)

⚠️ mzumsande opened an issue: "p2p: Inefficiency in block download / stalling algorithm"
(https://github.com/bitcoin/bitcoin/issues/32179)
When we were in a stalling situation during, we may kick a peer and download the block from another peer.
When we then receive the block, that will often result in many (hundreds) of blocks being connected in one go, resolving the stalling situation: A 1024-blocks window beginning at the new tip now has several open slots.

There is a bug in what happens next, which results in a less efficient IBD and possibly an unnecessary disconnection of a peer:

In the next `FindNextBlocksToDownload` call
...