💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2135407342)
No idea. [OPNsense](https://opnsense.org/about/about-opnsense/) is built on FreeBSD and I think the plugins just rely on whatever miniupnpd comes with the OS:

```
# pkg info miniupnpd
miniupnpd-2.3.3_3,1
Name : miniupnpd
Version : 2.3.3_3,1
Installed on : Thu May 2 10:44:17 2024 CEST
Origin : net/miniupnpd
Architecture : FreeBSD:13:amd64
Prefix : /usr/local
Categories : net
Licenses : BSD3CLAUSE
Maintainer : squat@squat.no
WWW
...

💬 luke-jr commented on pull request "doc: Change GiB to GB in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2135447798)
This is a duplicate of #29678 (without the accompanying code fixes)

👋 Sjors's pull request is ready for review: "validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553)

💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2135498664)
Force-pushed to address @TheCharlatan's (cosmetic) feedback: incorporated the clang-format-diff suggestions and removed a couple of unnecessary includes.

📝 fanquake opened a pull request: "[25.x] windeploy: Renew certificate"
(https://github.com/bitcoin/bitcoin/pull/30184)
Github-Pull: #30149
Rebased-From: 9f4ff1e9659597307f62510f1885ad8da3a1d9a3

💬 fanquake commented on pull request "contrib: Renew Windows code signing certificate":
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2135505873)
Backported to 25.x in #30184.

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1617508249)
Updated

💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617509629)
13.2 is from April 2023: https://www.freebsd.org/releases/13.2R/announce/
On FreeBSD it is easy and smooth to upgrade the OS (at least my experience since FreeBSD 4.x) even across major versions (e.g. 13 -> 14). Seems fine to only support >= 13.2.

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1617510788)
I've fixed that, plus increased the `min_required_fds` to also account for automatic outbounds in the last commit. I'm open to discuss about the latter, but I think it's something we should also account for

📝 fanquake opened a pull request: "guix: show `*_FLAGS` variables in pre-build output"
(https://github.com/bitcoin/bitcoin/pull/30185)
For example:
```bash
# ADDITIONAL_GUIX_COMMON_FLAGS set in the ENV
ADDITIONAL_GUIX_ENVIRONMENT_FLAGS="--emulate-fhs" ./contrib/guix/guix-build
<snip>
INFO: Building f751991 for platform triple x86_64-linux-gnu:
...using reference timestamp: 1716905119
...running at most 10 jobs
...from worktree directory: '/bitcoin'
...bind-mounted in container to: '/bitcoin'
...in build directory: '/bitcoin/guix-build-f75199182133/distsrc-f75199182133-x86_64-linux-gnu
...

✅ BenWestgate closed a pull request: "doc: Change GiB to GB in release-process.md"
(https://github.com/bitcoin/bitcoin/pull/30171)

👍 theuni approved a pull request: "feefrac: 128-bit multiply support in MSVC"
(https://github.com/bitcoin/bitcoin/pull/29758#pullrequestreview-2083318452)
utACK 5fb70b5b72c96a7e640900de56393b0bdb9df37a

(I didn't look into the additional unit tests)

Godbolt output here:
https://godbolt.org/z/8Msz984bv

Compiles down to roughly the same.

💬 theuni commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1617568664)
To help other reviewers:
```c++
__int64 _mul128(
__int64 Multiplier,
__int64 Multiplicand,
__int64 *HighProduct
);

Return value
The low 64 bits of the product.
```

So this should result in:
ret = <high, low>

Which matches the other impls.

💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1617616315)
Note the weird return value `__int64` here, which IMHO makes no sense whatsoever. Semantically, the return value is 2⁶⁴`HighProduct` + (uint64_t)`ret`.

🤔 sr-gi reviewed a pull request: "test/BIP324: disconnection scenarios during v2 handshake"
(https://github.com/bitcoin/bitcoin/pull/29431#pullrequestreview-2083422330)
I love it now 🚀

ACK ae528cb9e29b75d36a8018954f45bcfb886a4557

🤔 mzumsande reviewed a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2083413620)
Concept ACK

💬 mzumsande commented on pull request "net: log connections failures via SOCKS5 with less severity":
(https://github.com/bitcoin/bitcoin/pull/30064#discussion_r1617626592)
I think that the change in this line should be explained better / maybe be its own commit. `LogConnectFailure` is not related to SOCKS5 but also reached through `ConnectDirectly()`, so it's not clear to me from the commit msg or PR description why the log is changed.

📝 marcofleon opened a pull request: "fuzz: increase `txorphan` harness stability"
(https://github.com/bitcoin/bitcoin/pull/30186)
This moves `nNextSweep` from being a static variable in `LimitOrphans` to being a member of the `TxOrphanage` class. This improves the stability of the `txorphan` fuzz harness, as each orphanage (created every iteration) now has its own value for `nNextSweep`.

This PR builds upon https://github.com/bitcoin/bitcoin/pull/30170 which is still open, so it's a draft for now.

💬 dergoegge commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#issuecomment-2135762064)
rfm?

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617665519)
Right, that's good to know. However the current problem is that we can't compile this for *any* FreeBSD version. Looks like the `MLMSG` etc macros don't work in C++, because `typeof` is used.