💬 sedited commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2571742382)
It would be nice if these could stay where they are. How about the following:
<details>
<summary>Protected Method Wrapper</summary>

```diff
diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp
index 037d168e0e..04fbbc5473 100644
--- a/src/test/fuzz/block_index_tree.cpp
+++ b/src/test/fuzz/block_index_tree.cpp
@@ -14,0 +15 @@
+#include <test/util/validation.h>
@@ -44 +45 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree)
- Chainst
...

👍 dergoegge approved a pull request: "net: fix use-after-free with v2->v1 reconnection logic"
(https://github.com/bitcoin/bitcoin/pull/33956#pullrequestreview-3519080814)
Code review ACK 167df7a98c8514da6979d45e58fcdcbd0733b8fe

👍 sedited approved a pull request: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-3519081005)
ACK 6abbb6ed6cb29b20645a7ef78bcf2e51fc7bde49

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2571758136)
In `removeConflicts()`, we'd invoke `removeRecursive()` with a transaction that is in the mempool, and then this code would trigger. However I realize now that we already have the iterator that we're trying to remove, so I can save a map lookup by invoking the helper directly. (Prior to #33629 there were more places we'd invoke `removeRecursive()` on transactions in the mempool.)

Otherwise, I think this code will now only trigger in tests.

💬 Sjors commented on pull request "mining: fix -blockreservedweight shadows IPC option":
(https://github.com/bitcoin/bitcoin/pull/33965#discussion_r2571759611)
I have a larger refactor coming soon that simplifies this to `node.mining_args.default_block_reserved_weight`. But this is easier to backport.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2571763808)
I agree that this seems expensive, but an `Assume()` wouldn't save us anything, would it? I believe the code would still get invoked (unless the compiler is somehow smart enough to optimize it out).

📝 Sjors opened a pull request: "refactor: disentangle miner startup defaults from runtime options "
(https://github.com/bitcoin/bitcoin/pull/33966)
The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing.

They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of two distinct structs:

1. `BlockCreateOptions`: used by interface clients. As before, IPC clients have access to a safe / sane subset.
2. `MiningArgs`: these are set once during node startup

Both structs have a member for the maximum block height, which is not a p
...

💬 fanquake commented on pull request "refactor: replace manual promise with SyncWithValidationInterfaceQueue":
(https://github.com/bitcoin/bitcoin/pull/33962#issuecomment-3589855156)
You'll need to squash your commits: https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits.

💬 fanquake commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3589883475)
ACK 2e27bd9c3af91eb9fcc626fe65d065df0a80974d

🚀 fanquake merged a pull request: "ci: Add Windows + UCRT jobs for cross-compiling and native testing"
(https://github.com/bitcoin/bitcoin/pull/33764)

💬 laanwj commented on pull request "guix: reduce allowed exported symbols":
(https://github.com/bitcoin/bitcoin/pull/33950#issuecomment-3590126469)
> pretty sure this is atleast partly from https://github.com/bitcoin/bitcoin/pull/33181.

FWIW i cherry-picked commit 7b90b4f5bb10e2156709b07e3996f867e2421232 on top of b354d1ce5c47997aa2f93910e05c0f8daa8736eb (the commit before #33181). It finished succesfully. So it must have been something earlier even!

💬 sdaftuar commented on issue "Shallow invalid forks + ActivateBestChainStep result in overly aggressive mempool filtering":
(https://github.com/bitcoin/bitcoin/issues/32838#issuecomment-3590331276)
I discovered a related, but separate issue -- if we reorg to a lower-height chain, then I believe it's possible (likely?) for a mempool transaction with an unconfirmed parent (and which doesn't have BIP68 disabled) to be evicted from the mempool, even if the transaction is valid and would be reaccepted if immediately resubmitted.

My understanding is that this can happen due to the special heights we store for transactions with mempool dependencies ("tip height + 1") -- we can use a cached value
...

👍 sedited approved a pull request: "kernel: Expose reusable `PrecomputedTransactionData` in script validation"
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3520015994)
ACK 17d3575cfcb41adfd364e7a1912a4701786f7455

🤔 furszy reviewed a pull request: "refactor: replace manual promise with SyncWithValidationInterfaceQueue"
(https://github.com/bitcoin/bitcoin/pull/33962#pullrequestreview-3520076110)
ACK d56718838308c474cd73930ee276f1be7334dd68

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2572545917)
This vastly simplifies the logic, thanks!

📝 mzumsande converted_to_draft a pull request: "test: add functional test for outbound connection management"
(https://github.com/bitcoin/bitcoin/pull/33954)
For #29415, vasild added the possibility to have outbound connections made "naturally" by the node using addrman entries, by redirecting them via a SOCKS5 proxy to python p2ps or other full nodes, so that the node under test behaves as if it is connected to normal non-local peers from arbitrary networks, while we have full control over the peers.

I think this feature is really cool and want to suggest to integrate it more prominently into the test framework ("`auto_outbound_mode`") in the fir
...

💬 mzumsande commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#issuecomment-3590615136)
there are still some intermittent timeouts, will put in draft until I've fixed them.

👍 rkrux approved a pull request: "refactor: replace manual promise with SyncWithValidationInterfaceQueue"
(https://github.com/bitcoin/bitcoin/pull/33962#pullrequestreview-3520588660)
lgtm ACK d567188

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2572914108)
Thanks! Fixed in [f50c0d9a3e..f33eafff0f](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86).

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2572914368)
Thanks - fixed in [f50c0d9a3e..f33eafff0f](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86).