💬 l0rinc commented on pull request "scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/29641#issuecomment-3575743451)
Is this ready for review now?

👍 fanquake approved a pull request: "ci: Remove redundant busybox option"
(https://github.com/bitcoin/bitcoin/pull/33903#pullrequestreview-3505228993)
ACK fa0fee44a89c82750a39e9d54bb5a6fc72b77fce

💬 Sjors commented on issue "Mining interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3575819797)
In terms of interface changes I'd like to focus on #33819 `getCoinbase()` and #33922 `getMemoryLoad()` which add new functionality. The smaller breaking interface changes can wait until either we have a bigger breaking change or nothing else on our plate.

On the IPC side I don't really have priorities in mind, so maybe pick what's useful for @plebhash (as a stand-in for others who are both unfamiliar with the Bitcoin Core codebase and using other programming languages to connect).

💬 hebasto commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3575872453)
@m3dwards

> Not necessarily an issue with this PR but when trying to test on Trixie with `make -C depends HOST=x86_64-w64-mingw32ucrt -j9`
>
> I get: ...

Interesting.

I’m unable to reproduce this in a fresh Trixie container using Docker:
```
# apt update
# apt install git cmake curl make patch bison g++ ninja-build pkgconf python3 xz-utils g++-mingw-w64-ucrt64
# git clone https://github.com/bitcoin/bitcoin.git && cd bitcoin
# gmake -C depends -j 16 HOST=x86_64-w64-mingw32ucrt

...

💬 plebhash commented on issue "Mining interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3575893233)
we're doing a SRI release this week that is planned to include https://github.com/stratum-mining/sv2-apps/pull/59, and it's meant to support v30

but it will come with two major limitations, namely:
- the initial iteration of `bitcoin_core_sv2` crate is marked with a bunch of `todo`s that are workarounds for the lack of `interruptWait`, and are being tracked at https://github.com/stratum-mining/sv2-apps/issues/81
- running it on systems with low RAM for extended periods of time can lead to crash
...

✅ hebasto closed a pull request: "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable"
(https://github.com/bitcoin/bitcoin/pull/32409)

📝 hebasto reopened a pull request: "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable"
(https://github.com/bitcoin/bitcoin/pull/32409)
This PR adds a way to suppress the abort message box when running `test_bitcoin.exe` and `fuzz.exe` built with the debug runtime library on Windows.

Otherwise, a failing `assert()` triggers the `abort` routine, which displays a message box and causes a timeout in CI.

Here are CI jobs for the "Debug" configuration:
- on the master branch: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/14759346479/job/41435857332
- with this PR: https://github.com/hebasto/bitcoin-core-night
...

💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#issuecomment-3575923284)
Thanks for the review @TheCharlatan.
- Addressed [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554084325) on using post-condition checks.
- Dropped the change to use separate state for reporting errors and not invalidity as suggested by @TheCharlatan [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554092568)

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3576053570)
Rebased after https://github.com/bitcoin/bitcoin/commit/4f65a1c5db84426b9d814a18f63ac46632691ab2#diff-f14e9423bd43609969886b2e42751c55606344e0b71780564cdb23515802cec0R12, ready for review again.

🤔 rkrux reviewed a pull request: "Added rescan option for import descriptors"
(https://github.com/bitcoin/bitcoin/pull/31668#pullrequestreview-3505373212)
Concept ACK 6e523bda47854333d267f53cfa5292fd2eb40ec7

Agree with the intent to avoid rescanning if the user specifies so.

💬 rkrux commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2560222785)
I think the test cases can be greatly simplified with the following diff:

```diff
diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
index 49c41b0a1f..588d7725dc 100755
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -25,6 +25,7 @@ from test_framework.descriptors import descsum_create
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
+ assert_greater_th
...

💬 rkrux commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2560225153)
Would be good to add a small test case for this newly added condition as well.

💬 yuvicc commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3576089382)
Rebased.

📝 stickies-v opened a pull request: "kernel: don't use assert to handle invalid user input"
(https://github.com/bitcoin/bitcoin/pull/33943)
Crashing with `assert` for invalid user input is problematic for a library, e.g. this is impossible for FFI users to recover from, forcing them to re-implement the error handling logic client-side. See e.g. the [go](https://github.com/stringintech/go-bitcoinkernel/blob/88105c69b32c48919a45bb07fc05ca2c79d2728b/kernel/script_pubkey.go#L91-L102) and [.net](https://github.com/janb84/BitcoinKernel.NET/blob/4ba1ff802fe9a3b7c0af0012e6fdc33e5bc4f961/src/BitcoinKernel.Core/ScriptVerification/ScriptVerifi
...

💬 instagibbs commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3576309301)
ACK ade0397f59f2fb59ab0e4ebb39869ac343cc54ee

💬 fanquake commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#issuecomment-3576316460)
Concept ACK - @willcl-ark did you want to address any of the suggestions here?

💬 lucasbalieiro commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3576336813)
here are some insights from the experiments I’ve been running over the past few days.

When I first reported this issue (https://github.com/stratum-mining/sv2-apps/pull/59#issuecomment-3568252007], my server was also running a few utility tools that I typically use during long test sessions.

To eliminate the “maybe it’s something else eating the memory” hypothesis, I started a fresh session with **only** the following running:

* `sv2-apps`
* Bitcoin Core v30 (mainnet)
* The essential system pr
...

👍 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-3505760522)
reACK 2a64851c34fcf9955a673eb77463f68ab583a7bd

`git range-diff master b1581924a79e120f166719943646484ba00ff21b 2a64851c34fcf9955a673eb77463f68ab583a7bd`

💬 hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-3576356937)
> > @maflcko
> > > No objection, but assert/Assert seems to be used widely in the codebase, so shouldn't this be done for all test binaries, or none? Otherwise the same assert could lead to inconsistent behavior, depending on which test binary ran into it.
> >
> >
> > Thanks! Reworked.
>
> Thx, but it seems it would also need to be applied to bitcoind (or really all executables)?

It is already implemented for `bitcoind.exe` here:https://github.com/bitcoin/bitcoin/blob/5336bcd5784925c
...

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3576359913)
I restructured the implementation and commits a bit.


The `TxTemplateMap` now lives on the `NodeContext` rather than `MinerImpl` (interface). This reflects the fact that we want to track the global memory footprint instead of per client. It's a lightweight member `template_tx_refs` which should be easy to fold into a block template manager later.

It also made it easier to move `GetTemplateMemoryUsage` from `interface.cpp` to `miner.cpp`, where it's more reusable.

This in turn let me sp
...