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

👍 hebasto approved a pull request: "fs: remove `_POSIX_C_SOURCE` defining"
(https://github.com/bitcoin/bitcoin/pull/32460#pullrequestreview-2858336344)
re-ACK 24e5fd3bedcebacbc10f0449be61be636b77dd79, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/32460#pullrequestreview-2854183748).

💬 ryanofsky commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2898557463)
> We can't avoid splitting when calling `execvp`:

The suggestion is to let the shell do the splitting so we do not need to do it. We would call execvp with `sh` as the first command line argument `-c` as the second one, and the -signer value as the third one. This is also what the python subprocess module does when shell=True is passed.

💬 i-am-yuvi commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#issuecomment-2898614400)
Concept ACK

🤔 i-am-yuvi reviewed a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage"
(https://github.com/bitcoin/bitcoin/pull/32516#pullrequestreview-2858508507)
Concept ACK