💬 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

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2890914607)
I've added a commit to drop ` --enable-external-signer` from the global CI config.

💬 fanquake commented on pull request "cmake: Add missed `SSE41_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/32550#issuecomment-2890918551)
Just roll this into #32551?

💬 fjahr commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#issuecomment-2890931358)
Concept ACK

💬 Sjors commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2890932134)
> > Do we have a linter for unused includes?
>
> No includes are being removed here, so I don't see how that's relevant? IWYU would be the closest tool you could run.

The comment were useful for knowing when an include could be dropped (though not if they're incomplete).

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2095663048)
That would make the string_view point to the temporary returned by `substr()`. That temporary is destroyed after the end of the statement, so the string_view remains with a dangling pointer. Like in the example in https://en.cppreference.com/w/cpp/string/basic_string/operator_basic_string_view which says "// Bad: holds a dangling pointer".

Or a small standalone program to demonstrate:

```cpp
int main(int, char**)
{
std::string s{"abcd"};
std::string_view sv{s.substr(2)};
s
...

💬 fjahr commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2890950605)
Concept ACK

💬 fjahr commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#issuecomment-2890959492)
Concept ACK

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2095672887)
No, because when indented then it is rendered as a code block.

OK:
---

## 1. Run Bitcoin Core behind a Tor proxy

The first step is running Bitcoin Core behind a Tor proxy. This will already anonymize all
outgoing connections, but more is possible.

-proxy=ip[:port]
Set the proxy server. It will be used to try to reach .onion addresses
as well. You need to use -noonion or -onion=0 to explicitly disable
outbound access to onion services.

-proxy=ip
...

💬 fjahr commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#issuecomment-2890980318)
Concept ACK

I agree with the rationale, will test it shortly.

🤔 danielabrozzoni reviewed a pull request: "signet: omit commitment for some trivial challenges"
(https://github.com/bitcoin/bitcoin/pull/29032#pullrequestreview-2850728936)
ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8

I'm not too familiar with the signet miner, but the code looks good to me. I ran the tests locally, and did some manual testing using `-signetchallenge=51` and checking that the signet commitment is absent.

💬 pinheadmz commented on pull request "[28.x] Backport #31407":
(https://github.com/bitcoin/bitcoin/pull/32563#issuecomment-2891009384)
Concept ACK, starting guix build of this branch and will try to codesign with certificate