💬 kevkevinpal commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#issuecomment-2127147044)
Concept ACK [52506a0](https://github.com/bitcoin/bitcoin/pull/30156/commits/52506a0bb28a0c052deef9f484dc94e83de4f0f7)

Made a clean build of this change using
```
sudo make clean && ./autogen.sh && CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined && sudo make -j"$(($(nproc)+1))" install
```

## Running fuzz tests twice

(master: f15778536ad421f9805f0c005f0eb7b84dda1b4e)
```
FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_co
...

💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1611733528)
ok, makes sense why I only see it used in `cs_main` contexts, since it's a global mutex. thanks

💬 sdaftuar commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2127160378)
ACK 63cfa4ce88cac26abcacf3b243916e722ab17d75

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611597515)
Could make this slightly more readable with comments and two nullptr:

```cpp
if (sysctl(/*name=*/mib, /*namelen=*/sizeof(mib) / sizeof(int), /*oldp=*/nullptr, /*oldlenp=*/&l, /*newp=*/nullptr, /*newlen=*/0) < 0) {
```

🤔 Sjors reviewed a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2073809550)
Finished macOS review of QueryDefaultGatewayImpl.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611637097)
I guess you can't just do `for (const struct rt_msghdr* rt : buf)`

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611600021)
A useful hint to the reader what's going on here, and where it's documented:

```cpp
// The size of the available data can be determined by calling sysctl() with
// the NULL argument for oldp. See sysctl(3).
```

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611597943)
```cpp
if (sysctl(/*name=*/mib, /*namelen=*/sizeof(mib) / sizeof(int), /*oldp=*/buf.data(), /*oldlenp=*/&l, /*newp=*/nullptr, /*newlen=*/0) < 0) {
```

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611661588)
Do we want to check `rt->rtm_errno` first?

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611731634)
This seems quite brittle and I don't fully understand it. I did test that it seems necessary, e.g. doing `sa++;` causes it to not map IPv6 ports. If I look at `struct sockaddr_storage` I'm seeing `_SS_ALIGNSIZE (sizeof(__int64_t)`, but using `uint64_t` instead of `uint32_t` in your `ROUNDUP` also doesn't work.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611729605)
```cpp
// Skip sockaddr entries for bit flags we're not interested in,
// move cursor.
```

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611691990)
```cpp
// We only read from this address if a rtm_addrs bit flag is set.
```

💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#issuecomment-2127184039)
Added constant for https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1610126595
Also added a test that wallet does not signal v3 as it uses the default `CURRENT_VERSION` (this is correct as it wouldn't be safe to make transactions v3 before any of the network has adopted this change). I believe the `nVersion` can be changed via `CCoinControl::m_version`, though I'm not sure if that is accessible so I didn't write a test for it.

Rebased on top of #29873 + master (conflict with #29086)
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611755766)
i'm not sure either, i see it in some other libraries, but no alignment is mentioned in the manual page for `route` at all. Leaving out `ROUNDUP` works fine btw. Might just delete it.

🚀 achow101 merged a pull request: "test: improve robustness of connect_nodes()"
(https://github.com/bitcoin/bitcoin/pull/30118)

💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2127225225)
suggested changes were done, reviewed via `git range-diff master 7c3fb97 ef8de26`

💬 instagibbs commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#issuecomment-2127236772)
concept ACK

💬 murchandamus commented on pull request "policy: restrict all TRUC (v3) transactions to 10kvB":
(https://github.com/bitcoin/bitcoin/pull/29873#discussion_r1611774155)
> As CPFP carve out does not explicitly require a second child to actually
exist, it has the effect of granting a free +10KvB descendant size limit
when a single child is enough to bust the descendant limit.

That sounds like an unintentional behavior. Should that be addressed with a bug fix, or can we just get rid of the carve-out altogether in a world where we have TRUC transactions?

🤔 murchandamus reviewed a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2074103415)
crACK 154b2b2296edccb5ed24e829798dacb6195edc11

💬 glozow commented on pull request "contrib: Renew Windows code signing certificate":
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2127243747)
Backport for 26.x in #29899