💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609596900)
Done

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

I'm half tempted to bikeshed this to using `tools`, but this also seems totally fine to me :)

💬 maflcko commented on pull request "doc: Correct pull request prefix for scripts and tools":
(https://github.com/bitcoin/bitcoin/pull/30150#issuecomment-2124282415)
> I'm half tempted to bikeshed this to using `tools`, but this also seems totally fine to me :)

The bot will understand it, so you are free to use it. :)

💬 willcl-ark commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124284247)
There are also similar changes in https://github.com/bitcoin/bitcoin/pull/28418 but I didn't check this PR yet to see if they are the same.

📝 hebasto opened a pull request: "depends: Fetch miniupnpc sources from GitHub"
(https://github.com/bitcoin/bitcoin/pull/30151)
The https://miniupnp.tuxfamily.org website is unavailable now.

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1609607690)
Reverted to RO for group and others. I had misunderstood what you meant here:

> My point was more that we should probably not be making cookie files read-only to begin with. So only rw-r--r--, rw-r-----, and rw------- should be supported. Bringing us back to this maybe being best as -rpccookieperms=user/group/all rather than an octal modeset.

IMO it doesn't matter much if users and others get rw or ro, if they can read it, they can access the RPCs.

💬 TheCharlatan commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609609831)
How about:
```diff
diff --git a/contrib/devtools/bitcoin-tidy/example_nontrivial-threadlocal.cpp b/contrib/devtools/bitcoin-tidy/example_nontrivial-threadlocal.cpp
index 2b74df5d0e..b002712baf 100644
--- a/contrib/devtools/bitcoin-tidy/example_nontrivial-threadlocal.cpp
+++ b/contrib/devtools/bitcoin-tidy/example_nontrivial-threadlocal.cpp
@@ -2,0 +3,30 @@ thread_local std::string foo;
+thread_local char* bar;
+
+struct Test_Ok_1 {
+ int member;
+ Test_Ok_1() {
+ member
...

🤔 fjahr reviewed a pull request: "net: add ASMap info in `getrawaddrman` RPC"
(https://github.com/bitcoin/bitcoin/pull/30062#pullrequestreview-2070620568)
Code review ACK 2de765c25017e566777d927d86452d96082592a1

Feel free to ignore the nits but I can re-review quickly if you address them.

💬 fjahr commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609600761)
nit: This comment has a dot at the end while our convention seems to be to not add a dot.

💬 fjahr commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609607897)
nit: By alphabetical ordering this would go below the following line

💬 TheCharlatan commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2124300838)
Guix build (aarch64):
```
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
49447a196e
...

💬 fanquake commented on pull request "depends: Fetch miniupnpc sources from GitHub":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124309530)
> website is unavailable now.

Forever? Temporarily? Why not use the other official site?

Also no mention that this is now downloading a bunch of other stuff that we don't use?

💬 hebasto commented on pull request "depends: Fetch miniupnpc sources from GitHub":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124320183)
> > website is unavailable now.
>
> Forever?

I don't know.

> Temporarily?

I don't know.

> Why not use the other official site?

Which one? http://miniupnp.free.fr without HTTPS?

> Also no mention that this is now downloading a bunch of other stuff that we don't use?

Mentioned in the PR description.

💬 hebasto commented on pull request "ci: Add mising -Wno-error=maybe-uninitialized to armhf task":
(https://github.com/bitcoin/bitcoin/pull/30144#issuecomment-2124331610)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/207.

💬 maflcko commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609646449)
I don't have freeBSD to test this, but are you sure it doesn't crash when the dtor is there but empty?

In any case, what is the point of an empty dtor? Might as well remove it, or `modernize-use-equals-default` it?

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

👋 fanquake's pull request is ready for review: "guix: Use LTO to build releases"
(https://github.com/bitcoin/bitcoin/pull/25391)

💬 fanquake commented on pull request "p2p: detect addnode cjdns peers in GetAddedNodeInfo()":
(https://github.com/bitcoin/bitcoin/pull/30085#issuecomment-2124384604)
Backported to 27.x in #30092.

👋 fanquake's pull request is ready for review: "[27.x] Backports and probably finalize"
(https://github.com/bitcoin/bitcoin/pull/30092)

💬 josibake commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1609568747)
in https://github.com/bitcoin/bitcoin/pull/29680/commits/7debac0b09faf051229a4ead0b57e6658e00ac11 ("Change MemPoolRemovalReason to variant type"):

Can remove this line.