💬 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?

💬 dscotese commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1616266896)
I think it should too, but since you're on github, it makes sense to explore when expectations aren't met and propose solutions as we find them.

💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1616438227)
Looks like a slightly non trivial rebase. For context, which PR's triggered it?

💬 Sjors commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1616447514)
A bisect would be ideal. Though you could start with recent pull requests that touch net(_processing).cpp and were not backported to the v25.0 release. E.g. #27626 and #27625.

📝 JayBitron opened a pull request: "exclude ipc scheme from port check "
(https://github.com/bitcoin/bitcoin/pull/28020)
Previous PR https://github.com/bitcoin/bitcoin/pull/22087 cause new error, makes it impossible to use ipc protocol using zmq, this patch will exclude port checking on ipc urls.

💬 nberlin1980 commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1249523602)
This will remove `NET_UNROUTABLE ` and `NET_INTERNAL`, after which they cannot be re-added. Is that intentional (the function name implies it is intentional)? I ask because the other accessors guard against their removal and insertion.

💬 nberlin1980 commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1249529700)
Perhaps add test cases for `RemoveAll` as it relates to these two.

💬 mzumsande commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1616786805)
now the previously unaffected coinstatsindex_tests fails intermittently:
https://github.com/bitcoin/bitcoin/pull/27746/checks?check_run_id=14710590806
(doesn't seem to be related to the PR)
```
72023-07-02T07:40:59.796756Z (mocktime: 2020-08-31T15:34:test/util/index.cpp:15 IndexWaitSynced: Assertion `timeout > SteadyClock::now()' failed.
```

💬 Ayush170-Future commented on pull request "init: adding check for : for -torcontrol flag":
(https://github.com/bitcoin/bitcoin/pull/28018#issuecomment-1616813496)
Overall, I think the code is good. However, there have been some earlier remarks in this [comment](https://github.com/bitcoin/bitcoin/pull/23605#issuecomment-980045955) on where to place this code.

Someone with a better understanding of the entire codebase should comment on the placement of this.

In any case, I'm testing it locally and will report back on whether it meets the requirements soon.