💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2897976693)
`9cf56ad085...a8fcd8385e`: fix CI lint

💬 hebasto commented on issue "depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2897987043)
@John-zhan

You could use the same workaround as we use in the CI. See https://github.com/bitcoin/bitcoin/pull/32184.

💬 l0rinc commented on pull request "refactor: extract `DecodePushData` from `GetScriptOp` and `GetLegacySigOpCount`":
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2898016052)
@ryanofsky, @darosior, thank you for expressing your concerns.
I have removed the optimization part, I understand why that can be considered risky. The new rebase only contains extraction of the common script parts to avoid some work duplication and to help with understanding this part of the codebase better. It's also covered extensively with deterministic and random tests, comparing against fixed values, non-standard scripts, and completely random ones compared against the original implementa
...

👋 l0rinc's pull request is ready for review: "refactor: extract `DecodePushData` from `GetScriptOp` and `GetLegacySigOpCount`"
(https://github.com/bitcoin/bitcoin/pull/32532)

✅ l0rinc closed a pull request: "bench: replace benchmark block with more representative one (413567 → 784588)"
(https://github.com/bitcoin/bitcoin/pull/32457)

💬 l0rinc commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#issuecomment-2898025622)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/32554

👋 l0rinc's pull request is ready for review: "RFC: bench: replace embedded raw block with configurable block generator"
(https://github.com/bitcoin/bitcoin/pull/32554)

💬 laanwj commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100344020)
That's not how it's supposed to be. The default application codepage is something that the user can configure system-wide. The purpose of the applkcation manifest is to override it so it is always UTF-8 for the application. Exactly what the microsoft documentation says here: https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

This hasn't changed with newer windows versions, as far as i know, it's just that there is no usable UTF-8 codepage in earlier ones, so
...

📝 hodlinator opened a pull request: "headerssync: Keep tests ahead of increasing params"
(https://github.com/bitcoin/bitcoin/pull/32579)
### Motivation

Each release we increase the values of the Headers Sync constants `HEADER_COMMITMENT_PERIOD` and `REDOWNLOAD_BUFFER_SIZE` as per *doc/release-process.md*. In the next (v30) or following release, it is very likely that `REDOWNLOAD_BUFFER_SIZE` (`14827` as of v29) will exceed the `target_blocks` value used to test Headers Sync (`15000`).

This would result in the `HeadersSyncState::m_redownloaded_headers`-buffer not reaching the `REDOWNLOAD_BUFFER_SIZE`-threshold during the *sr
...

💬 jonatack commented on issue "test: `tool_wallet.py` references no-longer used CI":
(https://github.com/bitcoin/bitcoin/issues/32576#issuecomment-2898045057)
https://github.com/bitcoin/bitcoin/pull/28116 addresses it. Can update it.

💬 l0rinc commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#issuecomment-2898062472)
> It's not clear to me how the other commits are related to the PR description though

They were addressing the review comments I added to the mentioned PR, unifying naming (e.g. `fCacheCritical` vs `should_write`), extracting boolean to named symbol (`bool first_flush{m_next_write == NodeClock::time_point::max()`), adding helper for interval random generation. Not sure what the exact objections were, but I've removed them to keep only the logging now.

👍 pinheadmz approved a pull request: "test: allow all functional tests to be run or skipped with --usecli"
(https://github.com/bitcoin/bitcoin/pull/32290#pullrequestreview-2857855826)
ACK 7c13240d7b9a03b77ad84460e80007063367e05a

Trivial changes since last ack: rebase on master, adjust for removed legacy wallet

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 7c13240d7b9a03b77ad84460e80007063367e05a
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmgt3BwACgkQ5+KYS2KJ
yTr+QxAAmhg4rpylogoG5cCmOFvE5WovZU+Q63FoYp7SRBYt9abGf/OpmrAc5jbX
1JhfEqClryvZnIZPHNUdAfQxTXfT94+5AcD02/CYUpJO1++QwuKb
...

💬 laanwj commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100377809)
Well the idea was that by the time the compiler gets to it, a developer may already have done a frustrating search "what % expression do i need to use". Documenting it could avoid that (i don't think this particular blurb is "stale" in any way besides "not changed in a long time).

That said, there are tons of examples of `strprintf` in the code, and nothing uses length modifiers. Tinyformat also uses `%s` for strings without `c_str()`, which isn't explicitly documented.

👍 pinheadmz approved a pull request: "net: replace manual reference counting of CNode with shared_ptr"
(https://github.com/bitcoin/bitcoin/pull/32015#pullrequestreview-2857907230)
ACK 67bc008f62f9eac1616558770708c1e8547b09f0

Only rebase and trivial adjustments since last ack

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 67bc008f62f9eac1616558770708c1e8547b09f0
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmgt3x8ACgkQ5+KYS2KJ
yTqEKA//W/MMHBH002NCZEngJedxatJrtDCBj/RkfA9ijfOQ+WmNjnd2QNLJa3ds
z6NlZ0zlNNh60XZHGEgdWtEk8TvPECe/E9nEFjn6pze9REcIvizn/ZzomcNWgQxy
KJAptOo63YUXaK8mSQ
...

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2100412156)
Ah ok, any reason for the `+ i`?

💬 fanquake commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2898132974)
https://cirrus-ci.com/task/5032025251381248

💬 hebasto commented on pull request "subprocess: Handle quoted tokens properly":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2898150304)
> Alternatively, can't we just pass a vector of arguments to `RunCommandParseJSON` and avoid splitting/merging? Hmm maybe not as it comes in through one argument string `-externalsigner` anyhow, there's just some additional arguments that we add. OK this is annoying.

Exactly. We have to parse and tokenize the `-signer` argument in cases like `bitcoind -signer="/usr/bin/python3 /some_path/signer.py"`.

💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100435827)
> Tinyformat also uses `%s` for strings without `c_str()`, which isn't explicitly documented.

It *is* documented in this file ;)



> Well the idea was that by the time the compiler gets to it, a developer may already have done a frustrating search "what % expression do i need to use". Documenting it could avoid that

Ok, restored and clarified it for now. It will probably go away in 3 years with C++23 std::format

👍 fanquake approved a pull request: "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`"
(https://github.com/bitcoin/bitcoin/pull/32551#pullrequestreview-2857948411)
ACK 800b7cc42ca63f2a6b245a4d327c7092289da6e1

💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100437912)
> maybe suggest either...

thx, done