💬 maflcko commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2898213655)
> so it might result in a hidden performance regression instead of just a compile error.

I'd guess on modern, fast storage this doesn't make a visible difference. (Would be good to double check)

Then, given that the fallback seems to be needed by OpenBSD and newlibc (possibly others), it may be better to keep it?

Ref: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/15163923250/job/42638498492?pr=54#step:6:361 (OpenBSD 7.7):

```
src/util/fs_helpers.cpp:204:14: error: use
...

🤔 maflcko reviewed a pull request: "Fees: add Fee rate Forecaster Manager"
(https://github.com/bitcoin/bitcoin/pull/31664#pullrequestreview-2858027514)
(one more lint, also the ci test-each-commit failed)

💬 maflcko commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2100486155)

CachedMempoolForecast holds a cache of recent fee rate forecast. -> CachedMempoolForecast holds a cache of recent fee rate forecasts. [grammatical error: "forecast" should be plural when referring to multiple items]

🤔 pinheadmz reviewed a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408#pullrequestreview-2858010430)
rebase to f16c8c67bf to address comments, not touching the `rpcservertimeout` test just yet, discussion still ongoing

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100474952)
Ok I'll change the value here to 5 -- meaning the `sock.recv()` on L139 will always take that much time. That will, however, protect us against false positives on a slow CI, if the server regressed and actually responded to the requests out of order, but took more than 1 second to do so.

After we generate a block, the `sock.recv()` on L150 should execute very quickly, but will allow up to 5 seconds for a slow CI to respond to the two RPC requests. That applies to the `sendall()` as well.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100476656)
thanks, done

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100477997)
No, 4MB! Great catch thanks, fixed.

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100481900)
awesome yes, thanks, done

💬 pinheadmz commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2100488438)
yep thanks

📝 rkrux opened a pull request: "wallet, test: best block locator matches scan state follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32580)
Few follows-ups from #30221: Use `SetLastBlockProcessedInMem` more in `AttachChain`, add not null locator check in `WriteBestBlock`. Add log and few assertions in `wallet_reorgstore` test.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide cl
...

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2100509926)
What would you suggest as a test case?

💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2898284120)
Reverted to the prior change, given we cannot assume availablility.

💬 ryanofsky commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2898288496)
I'd think for both windows and unix, a good approach could be not parse -signer strings, just leave it up to the shell on unix, and shell+appilcation itself on windows (because on windows the application runtime is responsible for splitting arguments, usually using [CommandLineToArgv](https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw)) function. (This was also discussed pretty extensively in https://github.com/bitcoin/bitcoin/pull/13339)

If we can avo
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100551171)
Changed to "non-witness" and actually pushed it this time. :)

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100551433)
Meh. Then it would not make it consistent with MAX_STD_P2SH_SIGOPS, and having in the name that's it's a standardness rule is useful. I don't think it's worth changing.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100551574)
Done.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100551654)
Good call, done.

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2898331957)
> If we can avoid to have a split function at all that would seem great.

We can't avoid splitting when calling `execvp`:https://github.com/bitcoin/bitcoin/blob/87ec923d3a7af7b30613174b41c6fb11671df466/src/util/subprocess.h#L1382

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100561289)
Just varying the amounts because why not.

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100610295)
> In [1e826bf](https://github.com/bitcoin/bitcoin/commit/1e826bf91932e2d76969ae40a0c0ef85a990cf2f): Can the commit be updated to better explain why this is needed. Looking at mingw-w64, `GetACP` has returned `CP_UTF8` since it was introduced. I'm guessing the same is happening with MSVC (although that's less relevant given it doesn't effect releases)?

`GetACP` being compiled on Windows returns 1252, which is not the UT8 code page. And, basically, it would depend on the OS language settings.