💬 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.

🤔 rkrux reviewed a pull request: "rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures"
(https://github.com/bitcoin/bitcoin/pull/33014#pullrequestreview-3379921790)
Concept ACK 507501ddfa3f2c538a0817a5645b20ddde554362

> I don't think the corresponding bug is still present after v30, see: https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3443211089

I was able to reproduce this bug in the functional tests.

> Also, a test case in rpc_psbt.py would be good to verify the fix.

Consider adding the following test case in the second commit in this PR. The following test fails without this fix and passes with it.

```diff
diff --git a/test/functional/r
...

💬 rkrux commented on issue "Internal bug detected: FinalizeAndExtractPSBT(psbtx_copy, mtx)":
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3446588952)
Besides reproducing the cases mentioned in this issue via regtest, I was also able to reproduce the bug in functional tests and suggested adding it in the PR that contains the fix: https://github.com/bitcoin/bitcoin/pull/33014#pullrequestreview-3379921790

💬 maflcko commented on pull request "test: Use same rpc timeout for authproxy and cli":
(https://github.com/bitcoin/bitcoin/pull/33698#issuecomment-3446639814)
> 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 .

It shouldn't look "good". They should both timeout with the same timeout value.

> but why did we decide on half the timeout for CLI instead of using the same value?

The goal of this pull is to use the exact same value, which is also what it does.

⚠️ waketraindev opened an issue: "qt: rpc console, filtered commands replaced with '(…)' may execute unintended actions when recalled from history"
(https://github.com/bitcoin-core/gui/issues/907)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

In the Qt rpc console, when a command is filtered for privacy (`historyFilter`) it's parameters are replaced with `(…)`

For example:
```
createwallet "testwallet"

recalled as: createwallet(…)
```

If this filtered command is later recalled from command history (ArrowUp+Enter), it executes literally, creating or
executing an unintended command such as `createwallet(…)`

This will then cre
...

💬 maflcko commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2462779173)
This is a no-op. You assign the value and the expected value from the same function, and then assert that it is equal. This changes what the test is checking.

💬 yancyribbens commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2462796228)
Thanks for taking a look. The name doppelgangers comes from a similar test for BnB: https://github.com/bitcoin/bitcoin/blob/e744fd1249bf9577274614eaf3997bf4bbb612ff/src/wallet/test/coinselection_tests.cpp#L166.

I'm impartial on `res_a` vs `result_a`. I couldn't name it just `res` because there would be multiple assignments to `res` which the compiler complains about.

💬 hMsats commented on issue "Seemingly second (very long) validation at the same height":
(https://github.com/bitcoin/bitcoin/issues/33687#issuecomment-3446821485)
I'm now running/testing my node (starting cold) with:

```
std::chrono::seconds{consensusParams.nPowTargetSpacing}/10 * (BLOCK_DOWNLOAD_TIMEOUT_BASE + BLOCK_DOWNLOAD_TIMEOUT_PER_PEER * nOtherPeersWithValidatedDownloads))
```
in `net_processing.cpp`, where I introduced the division by a factor of 10. So based on 1 minute timeout instead of 10 minutes. When my node was warm, I could even base it on a 6 seconds timeout without problems (I've tested with 1 second to see if the timeout message was vi
...

🤔 furszy reviewed a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676#pullrequestreview-3380105952)
Concept ACK. Will review soon.

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3446886770)
rfm?

🤔 hebasto reviewed a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3380137916)
My Guix build:
```
x86_64
3de417ac1dade848d8b1609adf72c0faefcaf5acf03199259aa3aeb83e6a863f guix-build-59c4898994bd/output/aarch64-linux-gnu/SHA256SUMS.part
c6eda45fce2b34940ea53caa2acf0c1436122f89b85c0bc727834360f78a40f5 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu-debug.tar.gz
41584667134bfc5c35851d5005692c447d3fc529a310c7868030d6383d8bd840 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu.tar.gz
5c6d784bc03e59c
...