💬 vasild commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#discussion_r1609265987)
With this it will be possible to remove `export NOWARN_CXXFLAGS="-Wno-error=unreachable-code"` from dev environment on FreeBSD, thanks!

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2123884329)
> The current logic for file descriptor accounting is pretty convoluted and hard to follow.

Concept ACK, I stumbled on this recently in https://github.com/bitcoin/bitcoin/pull/29415

💬 BenWestgate commented on pull request "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2123904192)
@DrahtBot: I am fairly sure this PR is NOT a consensus change. It's a bugfix to a download verification tool script. Replace that label with a more accurate one.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2123952687)
> Concept ACK, kinda works.
>
> Minor nit noticed - could avoid adding ":0" as a port for I2P addresses.

thanks for the suggestion, did this.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2123989036)
> Another thing - list is currently comma separated, without wrapping. Wouldn't it be too long if somebody has IPv4, IPv6, CJDNS, Tor and I2P addresses at the same time?

Thanks for this point, it is fixed and the widget worswraps and extends when needed.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1609378065)
Thanks. Did the flow as you said. Now we are querying layer by layer. Please comment if it still needs improvement. I decided to pass the `std::map<CNetAddr, LocalServiceInfo>` to keep it flexible enough for different use cases; for example in this specific usecase (showing IPs in GUI), i was able to do check for "IsI2P" on CNetAddrs.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1609379939)
changed to a normal loop and checking for .begin().

💬 fjahr commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1609402573)
Sorry, I must have overlooked that if you already mentioned it before when I changed the type. I will address it if I have to retouch.

🚀 fanquake merged a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120)

💬 stratospher commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2124125832)
reACK 6629d1d.

💬 naiyoma commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2124129963)

> This is very close to @Brotcrunsher's original commit. At the very least they should be a co-author.

I've added the co-author.

📝 maflcko opened a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150)
`script` is confusing, because in the context of Bitcoin, it usually means Bitcoin script (c.f. `CScript` in `script.h`, or pull requests such as https://github.com/bitcoin/bitcoin/pull/27122 using the prefix).

This could be fixed by renaming it to `scripts` (with a plural `s` at the end), however, looking at the current usage `contrib` and `cli` seem more common (https://github.com/bitcoin/bitcoin/pull/29687, https://github.com/bitcoin/bitcoin/pull/26953, https://github.com/bitcoin/bitcoin/p
...

💬 maflcko commented on pull request "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124140010)
@BenWestgate I believe this is a typo in the documentation, fixed in https://github.com/bitcoin/bitcoin/pull/30150. (You can change the title yourself to `contrib:` and the bot will pick it up at some point.)

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2124149228)
> This PR still have a couple of unaddressed comments: [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998) and [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350).

Dear @hebasto , addressed all issues, updated the code & replied to queries. Thanks for the follow up.

👍 fanquake approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2070467640)
ACK fa3e1151a28345edff8f371283745bdd647f9a74

👍 fanquake approved a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144#pullrequestreview-2070470661)
ACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3 - many fixed with 13.x

💬 maflcko commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124165256)
Merge commits in pull requests are discouraged in this repo, please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

🚀 fanquake merged a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144)

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1609553954)
Clang actually expects this all the time, so just dsymutil is correct in depends as well (so we can't copy the bin as llvm-dsymutil) (this was the qt related issue I had seen before).

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2124214837)
Guix build (aarch64):
```bash
00becde2dd12878e3b9f50f27899a6a8b752343dade7c71781632715c3001473 guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/SHA256SUMS.part
a685b9cee54014e74639be1e8db2d55b7c008fdb3b31c1c708c364a49b56759a guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/bitcoin-e8c25e8a35e3-aarch64-linux-gnu-debug.tar.gz
d61228158409802e5aef11c39a0da5653a6c7e870d5f500483c32c75f319e8b6 guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/bitcoin-e8c25e8a35e3-aarch64-linux-gnu.tar.gz
49447a
...