💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2462078269)
Replying [this](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2451399250) comment:

Related concern about `btck_logging_set_options()` (which also existed before introducing the function): if a purpose for `cs_main` here is to protect the logging config variables, but logging operations read those same variables under `m_cs` (not `cs_main`), don't we have unsynchronized reads/writes? The config writes happen under `cs_main` while `LogTimestampStr()` and similar functions read `m_lo
...

🤔 AndricoSean reviewed a pull request: "test: Update BIP324 test vectors"
(https://github.com/bitcoin/bitcoin/pull/33688#pullrequestreview-3379176181)
lgtm

💬 Sammie05 commented on pull request "test: Use same rpc timeout for authproxy and cli":
(https://github.com/bitcoin/bitcoin/pull/33698#issuecomment-3445282493)
Tested PR #33698
I went through the new changes, built the code, ran the specific test p2p_headers_sync_with_minchainwork.py --timeout-factor=0.2 in both RPC and CLI modes and everything looks good . but why did we decide on half the timeout for CLI instead of using the same value?

📝 yancyribbens opened a pull request: "test: add case where `TOTAL_TRIES` is exceeded yet solution remains"
(https://github.com/bitcoin/bitcoin/pull/33701)
Show that `CoinGrider` halts searching when the number of attempts exceeds `TOTAL_TRIES`. To do so, show that a solution is found, then add one more entry to the same set of inputs. Since the search orders by `effective_value`, the solution is constructed such that only values with the lowest `effective_value` have the least weight. Only the lowest weight values will not exceed the `max_selection_weight`. Therefore, `CoinGrinder` will not evaluate all lowest weight solutions together before e
...

💬 yancyribbens commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#issuecomment-3445337171)
closes https://github.com/bitcoin/bitcoin/issues/33419

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3445467168)
> At most, I expect to receive a notification informing the user of that fact, but not actually stealing keyboard focus from the current application.

Not quite right:

> Consolatis │ dviola: I'd expect most compositors to raise and give keyboard focus to a surface requesting activation with a valid token (e.g. the surface the token originated from had keyboard or pointer focus at the time of issuing the token and not too much time has passed). I think some compositors treat "invalid" tokens
...

💬 ploo6669 commented on issue "SENDTEMPLATE Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/33691#issuecomment-3445975830)
the error is 1 bit video card
CPU has stable bytes

💬 maflcko commented on pull request "refactor/doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3446088430)
re-lgtm-ut-cr-ACK 1a1f46c2285994908df9c11991c1f363c9733087

👍 rkrux approved a pull request: "rpc: add optional peer_ids param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-3379820927)
ACK [8471074](https://github.com/bitcoin/bitcoin/pull/32741/commits/8471074fb996692fc56acbc34abe83103e084821)

👍 maflcko approved a pull request: "qt: add createwallet, createwalletdescriptor, and migratewallet to history filter"
(https://github.com/bitcoin-core/gui/pull/901#pullrequestreview-3379853438)
lgtm, seems fine to add. Left a question

💬 maflcko commented on pull request "qt: add createwallet, createwalletdescriptor, and migratewallet to history filter":
(https://github.com/bitcoin-core/gui/pull/901#discussion_r2462677504)
why add this, but not `importdescriptors`?

📝 maflcko opened a pull request: " contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO "
(https://github.com/bitcoin/bitcoin/pull/33702)
Historically, there was an attempt via `test/lint/lint-python-utf8-encoding.py` to enforce explicit UTF8 in every Python IO statement (`open`, `subprocess`, ...). However, the lint check has many problems:

* The check is incomplete and many IO statements lack the explicit UTF8 specification.
* It was added at a time when some systems were not UTF8 by default.
* The check is brittle, as it depends on a fragile regex.

In theory, now that the minimum Python version is 3.10 (since commit 212
...

💬 fanquake commented on issue "Unable to fuzz in local on MacOS 15.4.1":
(https://github.com/bitcoin/bitcoin/issues/33667#issuecomment-3446537462)
Looks like this could be https://github.com/Homebrew/homebrew-core/issues/235411.

💬 waketraindev commented on pull request "qt: add createwallet, createwalletdescriptor, and migratewallet to history filter":
(https://github.com/bitcoin-core/gui/pull/901#discussion_r2462710644)
When I was testing this patch, I ran into some exceptions in `importdescriptors` related to
the `internal` label and the `range` label (see bitcoin repo #31514, #32376, #33614, #33655)
and the command line history was helpful for manually editing to get it to pass succesfully;

At that point, I used #882 to manually clear the command history.

I wasn't sure about the commands that require a "desc" (descriptor) and allowed private ones,
so I held off until I could do some manual testing.

...

💬 waketraindev commented on pull request "qt: add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3446557710)
I'd appreciate any suggestions on discoverability of these console commands.

💬 maflcko commented on issue "Unable to fuzz in local on MacOS 15.4.1":
(https://github.com/bitcoin/bitcoin/issues/33667#issuecomment-3446561167)
So I guess this can be fixed by using the homebrew libc++?

💬 alvroble commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2462728387)
For naming consistency, this variable should be called `res`:
```suggestion
const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) {
```

💬 hebasto commented on pull request "ci: use pycapnp 2.2.1":
(https://github.com/bitcoin/bitcoin/pull/33693#issuecomment-3446577656)
The following diff also seems to work, unless I’ve missed minor issues or pitfalls already discussed in in https://github.com/bitcoin/bitcoin/pull/33201:
```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index a05e8f9648..1c69cc5437 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -157,11 +157,6 @@ jobs:
brew install --quiet python@3 || brew link --overwrite python@3
brew install --quiet coreutils ninja pkgconf gnu-getop
...

💬 alvroble commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2462730501)
Maybe it'd be better to use `res_a`/`res_b` instead of `result_a`/`result_b` and `available_coins` instead of `doppelgangers` for naming consistency.

💬 alvroble commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#issuecomment-3446581163)
Hey, I'm new to this project. So just learning. Ran the tests and they were not broken, and I agree with the rationale. Concept ACK.