✅ maflcko closed a pull request: "rev_32343_wip_nomerge_ci"
(https://github.com/bitcoin/bitcoin/pull/32535)

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2095516997)
// Unlike the block wide sigop limit, the BIP54 per
// transaction limit includes the prevout scriptPubKey.

📝 fanquake opened a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562)
We don't add or maintain these, and they are of little value, as
well as having the effect of polluting diffs, if changed.

They are also wrong, i.e `DEFAULT_SCRIPTCHECK_THREADS` is not in
`validation.h`.

💬 fanquake commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2095520803)
`libressl` ?

💬 maflcko commented on issue "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]":
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890705811)
> A duplicate of [bitcoin-core/gui#874](https://github.com/bitcoin-core/gui/issues/874)?

Haven't seen it, and created it here, because GUI CI failures affect this repo as well. Feel free to close, though.


I can't reproduce this locally, so maybe printing the rows on failure could help to debug this further via CI?

💬 fanquake commented on issue "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]":
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890712591)
I'd say leave it here, given it doesn't look like anyone is following up, or debugging, in the GUI repo.

💬 maflcko commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#discussion_r2095321942)
(same)

💬 maflcko commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#discussion_r2095532020)
forgot to remove the include?

```
[12:05:51.093] /ci_container_base/src/util/check.cpp should remove these lines:
[12:05:51.093] - #include <clientversion.h> // lines 9-9
[12:05:51.093]
```

However, the `CLIENT_NAME` symbol is still used in the kernel, so I am not sure removing the link dependency is meaningful. I'd say it should either be removed fully, or it can be kept. If you just want to turn the link-time dependency into a compile-time dependency, this is also possible, because
...

💬 maflcko commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#discussion_r2095321637)
```suggestion
LogWarning(STR_INTERNAL_BUG(strprintf("Internal bug detected: block %d has unexpected m_chain_tx_count %i that should be %i",
pindex->nHeight, pindex->m_chain_tx_count, prev_tx_sum(*pindex))));
```

nit: Could use the `STR_INTERNAL_BUG` helper, while touching this?

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2095540696)
Right. But I prefer to be explicit rather than feed `SetProxy()` an invalid value and rely on it doing the "right thing". It is not documented what `SetProxy()` would do with an invalid input, so I prefer to have these checks.

🤔 janb84 reviewed a pull request: "docs: clarify RPC credentials security boundary"
(https://github.com/bitcoin/bitcoin/pull/32424#pullrequestreview-2850500482)
re ACK https://github.com/bitcoin/bitcoin/commit/d6f622f36cfcf04d94f96ea360e346c5b9337be4

Changes since last ACK:
- Added text to clarify that there are additional flags to restrict RPC access

💬 laanwj commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890755002)
Yes, bisecting it would be neigh-impossible, given that it seems impossible to reliably reproduce. i already looked into the git history of `directory_iterator` in the `gcc/libstdc++-v3` and wasn't able to find any bug fix that would explain the behavior here, or how it was fixed. So it's likely something indirect.

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2095560957)
That would require some convoluted juggling with the parameters. Like:

1. `-proxy=addr:port=ipv6` sets the "ipv6" and "name" proxy, so that just a single `-proxy=1.2.3.4:5678=ipv6` works as expected. However:
2. `-proxy=0=ipv6` removes _only_ the "ipv6" proxy and not the "name" proxy, so that what you describe works: `-proxy=addr:port -proxy=0=ipv6`.

Another option would be to provide a separate name proxy, e.g. `-proxy=addr:port=name` or `-proxy=addr:port=dns`. But then users will probab
...

💬 Sjors commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2890768586)
It would be nice to also have a test (against an accidental soft fork) that fails if you change:

`MAX_STANDARD_TX_SIGOPS_COST{MAX_BLOCK_SIGOPS_COST/5 -1};`

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890791693)
The debug mode keeps state in additional structs/fields, so my bet would be that one of the states is multi-thread unsafe, but that is just a wild guess.

🤔 Sjors reviewed a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562#pullrequestreview-2850552831)
Concept ACK

Do we have a linter for unused includes?

No opinion on the seemingly unrelated `WIN32` changes.

💬 Sjors commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095577710)
3cf3695910f332f1aa8af57de7a01f63d5d7dc68: maybe change this in a separate commit?

Similar in other files, or is this auto generated?

💬 fanquake commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2890811590)
> Do we have a linter for unused includes?

No includes are being removed here, so I don't see how that's relevant?

💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2890847731)
> Do we use GNU extensions?

No, we don't.

👍 laanwj approved a pull request: "fs: remove `_POSIX_C_SOURCE` defining"
(https://github.com/bitcoin/bitcoin/pull/32460#pullrequestreview-2850606104)
ACK 0574aeaf6608dc3e118a4d2c0613dab2097cf62e