🤔 janb84 reviewed a pull request: "cmake: Introduce `WITH_PYTHON` build option"
(https://github.com/bitcoin/bitcoin/pull/31669#pullrequestreview-2966554130)
Concept ACK 40d935904c2b1319b863306d32fef235cd008a8e
(if we want to keep python check in CMAKE)

PR Adds build option to ignore / not fail on missing python. When python is missing or below required version the configuration will error out. This removes failing silently on python.
This PR will partial solve issue #31476 (strictly the Python issue)

---
Alternative thoughts:
Should CMAKE even test for python ? If you see CMAKE as configuration for the BUILD step and we do not use pytho
...

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3013076501)
@theStack I don't think this matters really, because "number of peers with at least one orphan" is used as a logistically-easier approximation for "number of peers which participate in transaction relay" (total memory usage is allowed to go up with more peers, because it's expected that peers that do transaction relay result in increases memory usage due to that). The fact that this means that peers which don't have any announced remaining orphans are not counted is a side-effect, not the goal.
...

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2172062756)
@yuvicc can you un-resolve this? I'd like other reviewers to see it and yell at me if I'm wrong :)

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3013114686)
> doing once more shouldn't introduce any noticeable overhead

Looking at `CCoinsViewCache::AccessCoin()`, it calls `FetchCoin`. This either returns a previously cached coin, or fetches it from its base (possibly on disk) and then adds it to the cache.

So the worst case overhead for a _second_ `AccessCoin()` call is having to perform another `try_emplace` on this cache.

The `CCoinsCaching` bench @l0rinc points to tests `AreInputsStandard` using a very small dummy coin cache. That's prob
...

👍 instagibbs approved a pull request: "test: Fix reorg patterns in tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587#pullrequestreview-2966618410)
reACK eb330b58d8d20bb4a5002906cb48bb03c5fa1a10

squashes and a couple errant comments deleted

🤔 scgbckbone reviewed a pull request: "test: Add `leaf_version` parameter to `taproot_tree_helper()`"
(https://github.com/bitcoin/bitcoin/pull/29371#pullrequestreview-2966639068)
ACK

💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2172077826)
For policy no way does this slowdown matter. That said, since we're already doing the computation in both policy and consensus contexts, maybe we can just reuse it? https://github.com/instagibbs/bitcoin/tree/2025-06-patho-bip54

it compiles

💬 fanquake commented on pull request "build: Find Boost in config mode":
(https://github.com/bitcoin/bitcoin/pull/32667#issuecomment-3013155618)
Guix Build:
```bash
f5e794c539bcf8260fe68be5893004f3a8f7aacbaba794704116b0fc38dd5813 guix-build-14653b869b91/output/aarch64-linux-gnu/SHA256SUMS.part
a701102c9ae49f32c780241a0dbfe16589213cbb43d7c48d8ca35de0c9032c2f guix-build-14653b869b91/output/aarch64-linux-gnu/bitcoin-14653b869b91-aarch64-linux-gnu-debug.tar.gz
e0f184484a32b5a00c8034a3cb3985e21abf6df342602e6ad4cd3063ccd12f2f guix-build-14653b869b91/output/aarch64-linux-gnu/bitcoin-14653b869b91-aarch64-linux-gnu.tar.gz
4ef01b549490fff7
...

💬 leopardracer commented on pull request "correct variable name in p2p_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/32816#issuecomment-3013168737)
> lgtm ACK

@maflcko everything is ok?

🚀 fanquake merged a pull request: "build: Find Boost in config mode"
(https://github.com/bitcoin/bitcoin/pull/32667)

💬 hebasto commented on pull request "miniscript, refactor: Make `operator""_mst` `consteval` (re-take)":
(https://github.com/bitcoin/bitcoin/pull/32564#issuecomment-3013197158)
Friendly ping @maflcko @theuni @darosior @fanquake who were reviewing https://github.com/bitcoin/bitcoin/pull/28657.

💬 fanquake commented on pull request "guix: warn and abort when SOURCE_DATE_EPOCH is set":
(https://github.com/bitcoin/bitcoin/pull/32678#issuecomment-3013209494)
Backported to 28.x in #32811.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where Base64 encoding is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3013213139)

> If possible it would be good to add python tests to check the new behavior

Sure, I'm also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists `=` padding or in between, so while creating a PSBT, I need to look at how I can achieve this. Or maybe by calling RPCs with a label.

> I also agree with https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2171811426 that this change doesn't need to be limited to base64 parameters and
...

💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3013238304)
@furszy I think it makes more sense to move the validation signals to their own scheduler thread if we are really concerned scheduled events will slow down block validation. `walletpassphrase` is currently the only RPC that schedules a new event, so all this PR will add to the queue are calls to `CWallet::Lock()` after the user needs a private key (and in the GUI an entirely different timer is used for this, based in Qt!).

We can also run some benchmarks, ie unlock 1000 wallets during IBD and
...

💬 sipa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3013244578)
The reason why `FeeFrac` supports negative fees and negative sizes is because that allows it to be used on "set differences" (*), which was at some point expected to be used inside the cluster linearization algorithm. It isn't in the current version in master, however, but it ended up being very useful in PR #32545 (see [this line](https://github.com/bitcoin/bitcoin/pull/32545/commits/68ce630abd669b86d9e2ff270aa2ba1e7332f95a#diff-1433c1fc4926a466291656ba67cf6b029523e4bd5da177ade812f25edf07343cR9
...

🤔 furszy reviewed a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2966805286)
ACK d06942c6731d5db7326bc565655b33a379a5d9b0

> I think it makes more sense to move the validation signals to their own scheduler thread if we are really concerned scheduled events will slow down block validation. walletpassphrase is currently the only RPC that schedules a new event, so all this PR will add to the queue are calls to CWallet::Lock() after the user needs a private key

Sounds fair to me.

💬 Sjors commented on pull request "rpc: Handle -named argument parsing where Base64 encoding is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3013274110)
> > If possible it would be good to add python tests to check the new behavior
>
> Sure, I'm also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists `=` padding or in between, so while creating a PSBT, I need to look at how I can achieve this.

You could just manually generate a random PSBT on regtest, hardcode it into a test and call `analyzepsbt`. It doesn't matter if it's unspendable.

Or you could even just call these methods w
...

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172171331)
I think it applies to `BOOST_REQUIRE_GT(result.pow_validated_headers.size(), 0);` as well

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172173668)
I don't like the new repetition - will have to debug it locally to understand why we run the condition outside of the loop in the first place 🤔

💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3013338371)
Code review re-ACK 9adf381d810f11b997ea036da6e2ce7dbad74cf6

Someone else with more experience should also review it, I have only lightly tested it locally.

💬 l0rinc commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2172225961)
you could store it as a multiplication instead of the comment:
```suggestion
HEADERS_DOWNLOAD_TIMEOUT_BASE_SEC = 15 * 60
```