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

💬 maflcko commented on issue "Running out of memory on a 2GB box - Initializing chainstate Chainstate [ibd] @ height -1 (null)":
(https://github.com/bitcoin/bitcoin/issues/31573#issuecomment-2885831745)
26.0 is EOL. Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

Did you compile a debug build? Are you using the release bins?

👍 hebasto approved a pull request: "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32525#pullrequestreview-2845701454)
ACK fa0a4473a88017e525829ecd3db47f536c35fb62.

Should a note about the broken MSVC 17.12 be added to the build docs?

💬 hebasto commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2885835138)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/32525.

✅ hebasto closed a pull request: "cmake: Restrict MSVC-specific workaround to affected versions"
(https://github.com/bitcoin/bitcoin/pull/32499)

💬 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-2885841787)
> `std::string_view` is known (and usable) at compile time as a view over some static data (it can be constexpr), so it can be concatenated to RPC docs. [1]

Also, I don't think this is true. I don't think it is possible in C++20 to concatenate two string_view into a single string_view at compile time.

💬 davidgumberg commented on pull request "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885848315)
ACK https://github.com/bitcoin/bitcoin/pull/32525/commits/fa0a4473a88017e525829ecd3db47f536c35fb62.

I would be partial to disabling this for MSVC 17.12 in the vein of #32499, I mean if it is known to be broken, and it's documented that it's broken, one might as well just disable it?

But, edge case of an edge case, so this seems fine to me.

💬 davidgumberg commented on pull request "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885854792)
reACK https://github.com/bitcoin/bitcoin/commit/fa0a4473a88017e525829ecd3db47f536c35fb62

📝 maflcko opened a pull request: "fuzz: Delete wallet_notifications"
(https://github.com/bitcoin/bitcoin/pull/32526)
The fuzz target has many issues:

* I has never found a meaningful issue.
* It is still slow, despite https://github.com/bitcoin/bitcoin/pull/28933 and https://github.com/bitcoin/bitcoin/pull/31238.
* It is unmaintained, see https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1814654792 (missing meaningful coverage) or https://github.com/bitcoin/bitcoin/pull/31238#issuecomment-2460821784 (unstable) or https://github.com/bitcoin/bitcoin/pull/31467#issuecomment-2672649759 (fix slowness)
...

💬 hodlinator commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092465291)
> whats the rationale behind multiplying by factor of 60?

Default timeout-factor is 1, so we get 60 seconds.

Here's a similar pre-existing example:

https://github.com/bitcoin/bitcoin/blob/c47f634718d4248fd2a30e51a57944f89da72a64/test/functional/test_framework/test_framework.py#L765-L773

> on master I dont think running the functional test with timeout-factor=99998 will trigger any failure.

99998 does fail feature_framework_startup_failures.py on master, but skipping back before th
...

✅ maflcko closed an issue: "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine"
(https://github.com/bitcoin/bitcoin/issues/27354)

💬 maflcko commented on issue "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine":
(https://github.com/bitcoin/bitcoin/issues/27354#issuecomment-2885915250)
closing for now. unless there are steps to reproduce how a real user can be affected by this, it may be best to postpone this for now.