💬 achow101 commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2885350354)
> It works much better, but there is still `fundrawtransaction` lock issue I think.

I don't think the locking is solvable without another large rewrite of the wallet. Currently, the wallet requires taking a mutex for exclusive access over many of its internal members, which necessarily means that generally only one wallet thing can happen at a time. So far, we've been focusing on the low hanging fruit of reducing the runtime of operations that are known to take a long time.

💬 gituser commented on issue "listunspent, fundrawtransaction, getwalletinfo locks wallet for any other operation":
(https://github.com/bitcoin/bitcoin/issues/27002#issuecomment-2885362613)
> I don't think the locking is solvable without another large rewrite of the wallet. Currently, the wallet requires taking a mutex for exclusive access over many of its internal members, which necessarily means that generally only one wallet thing can happen at a time. So far, we've been focusing on the low hanging fruit of reducing the runtime of operations that are known to take a long time.

Yeah, all your contributions are highly appreciated. Many thanks, hopefully these fixes will be merged
...

💬 davidgumberg commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2885370086)
> My bikeshed color: Since block hashes start with zeroes, maybe one could shard based off the last two bytes:

Just curious because the FAT32 file limit per-directory is so small, is there any scenario where a miner could DoS nodes with this format by also mining the last two bytes?

I think no, because at tip the additional hash needed for two bytes would be prohibitively expensive, although I'm not sure if there is a birthday-problem-like advantage because an attacker doesn't necessarily
...

📝 achow101 opened a pull request: "wallet: Remove watchonly behavior and isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523)
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.

Additionally, ISMINE_USED is removed as its use is only as a filter for cached balances. This PR changes the caching to utilize bools
...

🤔 shahsb reviewed a pull request: "util: C++20 `ToIntegral()` Improvement"
(https://github.com/bitcoin/bitcoin/pull/32522#pullrequestreview-2845493151)
Concept ACK

Approach ACK

🤔 shahsb reviewed a pull request: "util: C++20 `ToIntegral()` Improvement"
(https://github.com/bitcoin/bitcoin/pull/32522#pullrequestreview-2845494443)
LGTM. Indeed, good improvement.

Thanks for making the changes.

🤔 shahsb reviewed a pull request: "util: C++20 `ToIntegral()` Improvement"
(https://github.com/bitcoin/bitcoin/pull/32522#pullrequestreview-2845494857)
ACK https://github.com/bitcoin/bitcoin/pull/32522/commits/6135cc3f07ae91cab91927a2b610aaed2fb38858

💬 davidgumberg commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2885708663)
It might be better to use a more general solution for setting up VS prompts, this would also work once #32396 is merged.

```yml
- name: Set up VS Developer Prompt
shell: pwsh
run: |
$vswherePath = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe"
$installationPath = & 'C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe' -latest -property installationPath
if ($installationPath -and (test-path "$i
...

💬 maflcko commented on pull request "util: C++20 `ToIntegral()` Improvement":
(https://github.com/bitcoin/bitcoin/pull/32522#discussion_r2092382847)
not sure about adding docs that are already self-explanatory from the code without doubt. Adding such docs just means that two lines will have to be modified when modifying a trivial single line of code. Also, if the docs aren't needed, they are only a source of inconsistencies and typos, etc.

💬 maflcko commented on pull request "util: C++20 `ToIntegral()` Improvement":
(https://github.com/bitcoin/bitcoin/pull/32522#discussion_r2092378885)
not sure. We actually want C++26 erroneous behavior here, not ` [[indeterminate]] `, nor (zero-)initialized. Otherwise, it will become harder to diagnose bugs in the code with the compiler or with sanitizers.

💬 maflcko commented on pull request "util: C++20 `ToIntegral()` Improvement":
(https://github.com/bitcoin/bitcoin/pull/32522#discussion_r2092393107)
(realistically speaking in this very instance it probably doesn't make a difference and anything is fine, I just wanted to mention it for the general context.)

💬 maflcko commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092413854)
I'd say an alternative fix could also be to reduce the magic value from 99'999 to 9'999 or 999. ( Any value of that should be more than sufficient in practise. And anyone really wanting a larger timeout factor, can just set it trivially)

💬 davidgumberg commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2885771313)
> Tried removing the `compiler_has_bug`-logic completely and building with the version I had installed: [17.13.4](https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes-v17.13#17.13.4)...
>
> [...]
>
> ...did not encounter the internal compiler error?

My guess is either that a hotfix was pushed for `19.43`, or that this bug was already fixed in `19.43`, and the labels / bot response on the msvc bugtracker are inaccurate or canned, don't have an MSVC in front of me, b
...

⚠️ maflcko opened an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout)"
(https://github.com/bitcoin/bitcoin/issues/32524)
https://cirrus-ci.com/task/5094568045051904?logs=ci#L4419

```
[18:26:32.573] test_framework.authproxy.JSONRPCException: 'enumeratesigners' RPC took longer than 1200.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
```

Looking at the full log, the test is just idle and then times out. However, there is no indication why the RPC does not proceed.

💬 davidgumberg commented on pull request "refactor: bdb removals":
(https://github.com/bitcoin/bitcoin/pull/32511#issuecomment-2885785793)
crACK https://github.com/bitcoin/bitcoin/pull/32511/commits/fafee85358397289aa4c6b799d2603a5d89e83a2

📝 maflcko opened a pull request: "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32525)
Now that GitHub Actions has a fixed version and the Windows developers have updated their compiler, the workaround is no longer needed.

💬 Sjors commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2885815673)
Thanks everyone for their feedback. My current thinking is that there's two parallel ways forward:

1. Create a separate pool just for fee bumps, move those out of the extra pool
2. Harden the extra pool using the mechanism introduced in #31829

Fee bumps themselves are more DoS resistant than the other categories in extra pool. By moving them to their own structure (1), they can't be overcrowded by the other stuff. In that case there's no need to limit the extra pool; it's small, best case
...

✅ Sjors closed a pull request: "rfc: only put replaced txs in extra pool"
(https://github.com/bitcoin/bitcoin/pull/32510)

💬 maflcko commented on pull request "init: drop `-upnp`":
(https://github.com/bitcoin/bitcoin/pull/32500#issuecomment-2885816473)
> > An unknown upnp entry in settings.json would just be ignored?
>
> Confirmed.

Thanks for checking. I guess I was confused and the comment added in https://github.com/bitcoin/bitcoin/pull/31198/files was wrong?

review ACK 301993ebf7f8ec23050e91377e0fd05823bb372a

Also checked that unknown entries are ignored.

💬 maflcko commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2885823606)
> `std::string` no, it’s always a runtime, owning, heap‐allocated string, so it won´t work to produce the RPC docs.

Not sure what you mean by "won't work". What is the exact diff to reproduce the compile failure or error?