💬 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

👍 willcl-ark approved a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562#pullrequestreview-2850764439)
ACK e4aee0b3a23a9609e16bd7c8eda618dcf0a8b1db

💬 hebasto commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#discussion_r2095743353)
> I don't think so; MSVC is _only_ complaining about consteval literals inside `( ? : )` ternaries. I've pushed another attempt to avoid it by moving the subexpressions out of the ternaries.

FWIW, this bug was fixed in VS 17.9.

See https://developercommunity.visualstudio.com/t/Ternary-operator-breaks-consteval/10150913.

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

📝 hebasto opened a pull request: "miniscript, refactor: Make `operator""_mst` `consteval` (re-take)"
(https://github.com/bitcoin/bitcoin/pull/32564)
Same as https://github.com/bitcoin/bitcoin/pull/28657, but without the refactoring required to work around [fixed](https://github.com/bitcoin/bitcoin/pull/28657#discussion_r2095743353) MSVC bugs.

The second commit has been taken from https://github.com/bitcoin/bitcoin/pull/29167.

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

yes -_-

> However, the CLIENT_NAME symbol is still used in the kernel,

Mmh, that is done though the config header, and I am not sure how we should handle that yet. Seems like a separate problem to me? Then again the clientversion header is little more than some formatting helpers for some these variables. Could also remove `CLIENT_NAME` in the few cases where it is relevant, but `CLIENT_BUGREPORT` is probably useful to keep.

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

Done.

✅ hebasto closed a pull request: "cmake: Add missed `SSE41_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/32550)

💬 hebasto commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2891148659)
Add a commit from https://github.com/bitcoin/bitcoin/pull/32550.

🤔 hodlinator reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2849775833)
Code review 9406c17223da9df3a9875e4a5769a96ae66d61b8

Getting pretty close to ACK. One main inlined question about error handling in dbwrapper.cpp.