👍 luke-jr approved a pull request: "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting"
(https://github.com/bitcoin/bitcoin/pull/27411#pullrequestreview-1508023222)
utACK e7cf8657e1165ea5da3911a9e543837cd8938f97

Approach: Unsure about the refactoring, but ok

💬 luke-jr commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#discussion_r1248375619)
It's the pointer that's `const` here. Seems pretty reasonable, though it only has a net effect of preventing accidentally changing `wallet_model` in this method.

💬 luke-jr commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#discussion_r1248375640)
The `const` has no effect here.

💬 tuanggo commented on issue "ThreadI2PAcceptIncoming temporarily bypasses 125 connection ceiling":
(https://github.com/bitcoin/bitcoin/issues/27843#issuecomment-1615739130)
ok

💬 ajtowns commented on pull request "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings":
(https://github.com/bitcoin/bitcoin/pull/27401#issuecomment-1615756925)
utACK

Manually setting that flag by overriding CXXFLAGS disables a whole bunch of debugging warnings, so handling it directly seems very helpful.

💬 pablomartin4btc commented on issue "Unable to open descriptor wallets that are not in a wallet directory":
(https://github.com/bitcoin/bitcoin/issues/26739#issuecomment-1615886802)
> I'm not sure that this is something that we should implement. This issue is primarily for brainstorming and comment gathering.

I understand, and just for my own sake, I'd need to play around with the code and see how it behaves respect to the issues described above in @ryanofsky's comment, plus the "downgrading issues" discussion between both of you, and also the conversations around situations detailed on both PRs #1889 and #11687.

💬 darosior commented on pull request "Miniscript: always treat unsatisfiable scripts as insane":
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1615891160)
I know, my point is the definition as spelled out before would still hold (anybody would be able to spend it but the "apparent policy of this miniscript match its actual semantics"). What i was trying to get at is that the rationale for requiring sane miniscripts to contain a signature check isn't to make the actual semantics match the apparent policy, but that it's unusable otherwise. And it's the same for unsatisfiable miniscripts.

Anyways, i was just nitpicking. I pushed an update to leave
...

💬 darosior commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1615894814)
I think the `CHECK_NONFATAL` that we don't parse Miniscript descriptors with an empty set of keys can be removed here, No need to hold off this PR and it was added in https://github.com/bitcoin/bitcoin/pull/27997 which fixes the bug it uncovered.

💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1615962850)
Hey there, I'm a beginner contributor and would like to contribute to this, but it'd take me some time to understand what this issue is about. Is that alright?

💬 JayBitron commented on pull request "Validate port-options":
(https://github.com/bitcoin/bitcoin/pull/22087#issuecomment-1615975384)
@fanquake We now have new error: Error: Invalid port specified in -zmqpubhashblock: 'ipc:///root/test/zmq.block.hash'
Error: Invalid port specified in -zmqpubhashblock: 'ipc:///root/test/zmq.block.hash'

💬 apoelstra commented on pull request "Descriptors: rule out unspendable miniscript descriptors":
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1615982610)
> What i was trying to get at is that the rationale for requiring sane miniscripts to contain a signature check isn't to make the actual semantics match the apparent policy, but that it's unusable otherwise

No, the rationale was definitely about transaction malleability, and about transactions being spendable in surprising ways. Transactions *not* being spendable, given a policy which indicates non-spendability, has never been something we've tried to prevent or even thought much about.

I'
...

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1248935619)
Thanks! I've incorporated that here.

🤔 luke-jr requested changes to a pull request: "Switch RPCConsole wallet selection to the one most recently opened/restored/created"
(https://github.com/bitcoin-core/gui/pull/696#pullrequestreview-1509051570)
utACK 5781f45ff32841a3b5d365f5683def34ff2c356e, after method renamed to `setCurrentWallet`

💬 nberlin1980 commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1248977166)
```suggestion
if blockmintxfee_btc_kvb > 0: # can't go below zero-fee
```
To match the previous suggestion?

⚠️ brunoerg opened an issue: "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated"
(https://github.com/bitcoin/bitcoin/issues/28019)
Due to changes to the functions `V1TransportDeserializer::readHeader` and `V1TransportDeserializer::GetMessage`, the "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" section in `fuzzing.md` is outdated. The `git apply` command doesn't work.

```sh
$ mkdir bitcoin-honggfuzz-p2p/
$ cd bitcoin-honggfuzz-p2p/
$ git clone https://github.com/bitcoin/bitcoin
$ cd bitcoin/
$ ./autogen.sh
$ git clone https://github.com/google/honggfuzz
$ cd honggfuzz/
$ make
$ cd ..
$ CC=$(pwd)/
...

💬 luke-jr commented on pull request "Remove confusing "Dust" label from coincontrol dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1616149064)
Concept ACK, but I see no change from the second commit. Only the coincontrol dialog's "Dust" field disappears - nothing else differs in my screenshots.

💬 luke-jr commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1248991477)
Probably not good to ignore it never returning... it's leaking resources. Maybe kill it and/or tell the user somehow? :/

💬 luke-jr commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1248993583)
Won't this still make an externally-visible DNS request?

💬 Crypt-iQ commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1616218854)
I guess the issue is that I figured depends would work out of the box without me trying to mess with anything?

💬 Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1616244103)
> [Thread 0x7ffec0f4b700 (LWP 1828755) exited]
> bitcoin-qt: httpserver.cpp:223: http_request_cb(evhttp_request*

Do you happen to know what libevent version you were on?