👍 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
...

💬 fanquake commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1609567748)
I agree, this change doesn't look like an improvement.

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1609570161)
style nit: The code should be self-explanatory, so could remove this comment? (This should also re-trigger the false positive failed CI)

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124254747)
> The only TODO that is left to do is the Qt settings migration

Avoiding this complexity is another reason to add a NAT-PMP fallback, then we'd be right to keep the option named the same and only change the descriptions.

💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2124274889)
Force-pushed:

- Changed `mapped_as` and `source_mapped_as` to optional.
- Changed description of the fields as suggested by @fjahr.
- Reordered these new fields (https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1607941908)

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