💬 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

📝 maflcko opened a pull request: "Use WitnessV0KeyHash in TestAddAddressesToSendBook"
(https://github.com/bitcoin-core/gui/pull/875)
Fixes https://github.com/bitcoin/bitcoin/issues/32558
Fixes https://github.com/bitcoin-core/gui/issues/874

This fixes a bug introduced in commit fafee85358397289aa4c6b799d2603a5d89e83a2, which changed the type of the dummy address from `WitnessV0KeyHash` to `PKHash`. It was expected that this is fine, given that this is just a dummy address. However, the base58 characters can include the substring "io", leading to test failures later on.

Fix it by just using `WitnessV0KeyHash` again.

F
...

💬 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-2890853944)
(created fix)

💬 willcl-ark commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2890875967)
Concept ACK.

https://github.com/bitcoin/bitcoin/blob/ff1ee102c4babadd85e4a18234061f10daaa119d/src/validationinterface.h#L11
https://github.com/bitcoin/bitcoin/blob/ff1ee102c4babadd85e4a18234061f10daaa119d/src/cuckoocache.h#L10

The only others I found were in subtrees.

💬 fanquake commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2890881787)
Thanks, dropped.

💬 maflcko commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2095633829)
> Is there really a scenario where we could be reading a block, but not have its block index entry?

I can't see any, but this would require some more refactors

📝 fanquake opened a pull request: "[28.x] Backport #31407"
(https://github.com/bitcoin/bitcoin/pull/32563)
Backports #31407 + #32003.

🤔 Sjors reviewed a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530#pullrequestreview-2850636522)
Concept ACK

If only to remind ourselves that 32-bit systems exist.

Before #28358 we automatically rounded down the number, which wasn't great.

💬 Sjors commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095631168)
Maybe clarify this with something like:

```cpp
constexpr is_32bit{sizeof(void*) == 4};
```

💬 Sjors commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095631253)
ea561779abfb2298a9f20ceafa9e05fe2c58d841: let's just use 4096? This setting doesn't check actual memory capacity on 64-bit systems either, so we don't need to be this protective.

💬 Sjors commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095634131)
ea561779abfb2298a9f20ceafa9e05fe2c58d841: as with `-maxmempool`, I don't think we should be too "helpful" and just limit it to the physical maximum.

💬 fanquake commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2890884936)
I've opened something (not-yet-fully-tested) here #32563 for `28.x`.

💬 fjahr commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2890891024)
Concept ACK

I kind of like the idea but seems way too much effort to actually maintain this in a consistent way for very little gain. I don't remember if any of these ever saved me time, probably not because my workflow doesn't rely on them and most people probably use an editor that allows them to jump to definition.

💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-2890893688)
Concept ACK

💬 fjahr commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#issuecomment-2890904040)
Concept ACK