💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2135121750)
FWIW, i'm not seeing that problem on TurrisOS (openwrt) with miniupnpd 2.3.3. Renewals work fine, both with 20 minute and 5 minute reannounce period. So it's either something that was solved in the meantime, or something different in your networking situation.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617187166)
Fixed.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617189498)
It's an invariant, but it was poorly described, which I've hopefully addressed now (see `m_offset` docstring).

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617190116)
I don't think so, since that case doesn't let us avoid the loop.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617190282)
Good catch, done.

💬 cbergqvist commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1617190868)
I'd consider SSE supporting CPUs quite common. Seems like using such instructions can offer efficiency gains but are sufficiently complex to write papers about - https://stackoverflow.com/a/42675620, something for the future maybe.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617193263)
I think it's better to match `std::vector` behavior here (technically, spec doesn't say anything about the capacity of a copied element, but common implementations make it just big enough to hold the copied data, I believe).

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617193395)
Fixed.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1617197710)
Fixed!

💬 Sjors commented on pull request "doc, rpc: Release notes and follow-ups for #29612":
(https://github.com/bitcoin/bitcoin/pull/30167#issuecomment-2135163999)
I checked that efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 still gives me the same snapshot as the last the time I reviewed #29612.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1617216061)
Thanks, will try.

💬 Sjors commented on pull request "ci: Reintroduce fixed "test-each-commit" job":
(https://github.com/bitcoin/bitcoin/pull/28497#discussion_r1617224001)
> because github treats pull requests as being independent and isn't aware of relationships between them.

On the fork I tested on the pull request wasn't against `master` but against branch `A`.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2135196336)
Both our setups are at miniupnpd 2.3.3; the plugin description incorrectly says 2.3.1: https://github.com/opnsense/plugins/issues/4003#issuecomment-2133421334

So perhaps there's a difference in how miniupnpd controls the router / firewall itself, which causes the renewal to fail on my end. This is above my pay grade...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2135219680)
i've been trying to build the package myself, and there's miniupnpd-nftables and miniupnpd-iptables, do you know which one you have? i see i have -iptables installed at the moment, it appears.

💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1617205722)
Unfortunately these types of descriptions pretty much break `help2man`'s detection algorithms, see images in initial post.

💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1617204105)
IMO the former provides more information (that unused params aren't needed)

💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1617249137)
fixed in 61e73920d8

💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1617250283)
Taken in 61e73920d8

💬 willcl-ark commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1617249793)
I think it's helpful to detail the only command.

💬 Sjors commented on issue "Pause IBD during AssumeUTXO snapshot load":
(https://github.com/bitcoin/bitcoin/issues/29993#issuecomment-2135225915)
I loaded a mainnet snapshot today with a pruned node with limited --dbcache=10000 on a 2019 MacBook Pro. This time I had the node sync for about an hour before loading the snapshot.

At some point the log says:

```
2024-05-28T12:38:11Z Cache size (10375257872) exceeds total space (10370521497)
2024-05-28T12:38:11Z FlushSnapshotToDisk: flushing coins cache (10375 MB) started
```

This flush took 50 minutes, while the chain synced from 392,803 to 417,312. It then continues loading coins
...