💬 achow101 commented on pull request "build: Re-enable external signer on Windows":
(https://github.com/bitcoin/bitcoin/pull/25696#issuecomment-1461152815)
ACK 1a0d8e178c7b9a3e94d14f94b77305802ebdc93b

🚀 achow101 merged a pull request: "build: Re-enable external signer on Windows"
(https://github.com/bitcoin/bitcoin/pull/25696)

👋 jonatack's pull request is ready for review: "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs"
(https://github.com/bitcoin/bitcoin/pull/27231)

💬 amitiuttarwar commented on pull request "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`":
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1461221191)
are there situations where you'd recommend a node operator to use `-maxconnections=0` over `-connect=0` or `-noconnect`? I find the current behavior with `-maxconnections=0` to be odd how the node just stalls with very little guidance for the user.

I can't think of cases where I'd recommend an operator to set `-maxconnections` to be less than 8 or 10 for security reasons. If someone was seeking a lower bandwidth environment, I'd recommend disabling automatic connections & having manual truste
...

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1461229059)
to minimize the complexity that we need to maintain over time, we try to reduce the amount of client-side niftiness that we offer. it seems to me that the string would offer all the information needed for a client to write a simple query if they wanted the sum of multiple. agreed that the syntax is another challenge of the array type

so I am +1 to leaving as is

💬 fanquake commented on pull request "build: Re-enable external signer on Windows":
(https://github.com/bitcoin/bitcoin/pull/25696#issuecomment-1461495567)
post-merge Concept NACK. I don't know why we are leaning back into this.

💬 S3RK commented on pull request "wallet: 25806 follow-up":
(https://github.com/bitcoin/bitcoin/pull/27227#issuecomment-1461505650)
Code review ACK 475c20aa568d597c7850c784058596ae26f37496

💬 stratospher commented on pull request "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`":
(https://github.com/bitcoin/bitcoin/pull/26899#discussion_r1130625017)
> Historical reasons and no one bothered yet to make this consistent with the other arguments?

most possible reason :)

💬 1440000bytes commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1461538006)
Concept ACK

💬 vasild commented on pull request "gui: use the stored CSubNet entry when unbanning":
(https://github.com/bitcoin-core/gui/pull/717#discussion_r1130639712)
Yes, I noticed the same - looks like we don't need a loop here since it is always just one being selected. But it was like this and I left it as is (with a loop) to minimize the amount of unnecessary changes that can have unexpected adverse effects. I do not know why the GUI wouldn't let me select more than one row. Maybe a change elsewhere in the code would allow that and then this code would be broken without a loop.

👍 1440000bytes approved a pull request: "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`"
(https://github.com/bitcoin/bitcoin/pull/26899)
utACK https://github.com/bitcoin/bitcoin/pull/26899/commits/4e9d06bf22da5e972c541b61036ffda73a51a114

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#issuecomment-1461577699)
> Nit - there is typo in commit message / PR title (s/docment/document/).

Thanks, i've updated this in 65e3abcbf

💬 MarcoFalke commented on pull request "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1130705346)
```suggestion
for (const auto& c:categories.get_array().getValues()) {
const std::string& c{c.get_str()};
```

no need to create three copies of the same thing

💬 kristapsk commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#issuecomment-1461643692)
> > Nit - there is typo in commit message / PR title (s/docment/document/).
>
> Thanks, i've updated this in 65e3abcbf

You should also edit PR title. It is used by [github-merge.py](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/github-merge.py).

💬 fanquake commented on pull request "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1461655213)
> or none are passed, including not recognizing none:

Any idea when/how this was broken?

💬 willcl-ark commented on pull request "doc: document json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#issuecomment-1461659016)
>

Thanks, I thought that updated from the force push, but apparently not. Now also done :)

💬 fanquake commented on pull request "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1461666350)
Probably in f1379aeca9d3a8c4d3528de4d0af8298cb42fee4 from #25614:
> replace the unused BCLog::Level:None string "none" with an empty string
as the case will never be hit

So only broken for 24.x.

💬 willcl-ark commented on issue "wallet: balance gone when tx broadcast failed":
(https://github.com/bitcoin/bitcoin/issues/20943#issuecomment-1461668624)
> ...the balance is (re)computed by examining all txs and adding up the outputs that belong to the wallet.

indeed

> One of the criteria is that unconfirmed transactions need to be in our mempool to be counted as part of the "trusted" balance. A transaction which failed to make it into the mempool will result in the wallet recording that the parent has had all of its outputs spent (so it does not increase the balance), but that the child transaction is not trusted, so change back is not cou
...

💬 MarcoFalke commented on issue "Many sendcmpct messages are sent during UpdateActiveChain()":
(https://github.com/bitcoin/bitcoin/issues/21903#issuecomment-1461683306)
The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest and direction. Pull requests with improvements are always welcome.

✅ MarcoFalke closed an issue: "Many sendcmpct messages are sent during UpdateActiveChain()"
(https://github.com/bitcoin/bitcoin/issues/21903)

💬 MarcoFalke commented on issue "Mac OS latest Bitcoin core latest release will not run as Mac OS, "you can't open the application 'Bitcoin core'"":
(https://github.com/bitcoin/bitcoin/issues/25834#issuecomment-1461685951)
No, I think the error message is different. But there is no further replies from OP available, so closing for now.

Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?