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

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100618966)
It's confusing, but otherwise harmless.

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-2898436841)
ACK 80d213677962268a82d65d8915c518f0c02867d9

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100633125)
> Looking at mingw-w64, `GetACP` has returned `CP_UTF8` since it was introduced.

I see it differently. I've cross-compiled this code:
```c++
#include <iostream>
#include <windows.h>

int main()
{
std::cout << GetACP() << "\n";
}
```

and it prints 1252 on Windows.