💬 glozow commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613660947)
```suggestion
// Create entry for this object in g_tracker and populate m_track_entry
void Register()
```

💬 glozow commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613661502)
```suggestion
// Get value corresponding to this object in g_tracker
std::optional<uint64_t>& Deref()
```

💬 glozow commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613637296)
There's an assumption that at least one of the operations is applicable every time. Pretty obviously true but I assigned variables to all the possible conditions while reviewing and used it to add an assertion. Might be easier to read so I figured I'd leave a comment.
```suggestion
const bool non_empty{num_buffers != 0};
const bool non_full{num_buffers < MAX_BUFFERS};
const bool partially_full{num_buffers > 0 && num_buffers < MAX_BUFFERS};
const bool multiple
...

⚠️ BenWestgate opened an issue: "Welcome screen uses GiB values as GB, risking running out of space"
(https://github.com/bitcoin-core/gui/issues/821)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The welcome screen correctly displays "55 GB of space available" while my system has 54322808 bytes available.

But it incorrectly says "... 12 GB of data will be stored in this directory" or "610 GB of data" if I uncheck the 2GB pruning. The values it is adding and comparing do not have the same unit. One is a GUI prune setting and/or space available in GB, the other is actually GiB: h
...

🤔 glozow reviewed a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2077346481)
Concept ACK!

💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613746327)
Hmm, this is a good point. The `RingBuffer` class' interface isn't a ring buffer, but a deque; the implementation happens to use a ring buffer. Suggestions/bikeshedding for a better name welcome.

💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613757739)
"Integral in Rage" sounds like a high-school metal band.

💬 fjahr commented on pull request "doc, rpc: Release notes and follow-ups for #29612":
(https://github.com/bitcoin/bitcoin/pull/30167#discussion_r1613761372)
My thinking here was that if we don't know our own network then something must be really wrong and it's probably a developer error. But I guess an exception is fine, so I removed the assert.

💬 fjahr commented on pull request "doc, rpc: Release notes and follow-ups for #29612":
(https://github.com/bitcoin/bitcoin/pull/30167#discussion_r1613761550)
done

💬 sr-gi commented on pull request "Low-level cluster linearization code":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2129982443)
Some more benchmarks.

MacBook M3 Max:

| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1,678.30 | 595,841.56 | 0.4% | 1.11 | `LinearizeNoIters16TxWorstCase`
| 4,975.16 | 200,998.49 | 0.1% | 1.10 | `LinearizeNoIters32TxWorstCase`
| 10,624.85 | 94,118.94 | 0.1% | 1.10 | `LinearizeNoIters48TxWorstC
...

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1613772440)
Yes! That's why the next commit removes it. The reason for the split up into 2 commits is that I wanted to make the previous commit as "move-only" as possible so it is easier to review (especially with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`).

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1613775486)
yes, the proper way would be to set a deadline with a monotonic clock, instead of using the same timeout every time
though, everything considered, the gateway can only hold up this thread; it's not like the rest of bitcoind is waiting on it, and if the gateway is not to be trusted then it's not like we're going to get any useful mappings anyway

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1613776595)
True - on one hand this should make it minimally faster - on the other hand having simpler function signatures is also nice. I don't think it really matters much either way, parsing an arg once more can't be very expensive and this is not on any critical path anyway, so I kinda like the current way more.

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1613776822)
see previous comment

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1613777848)
Good point! The 30s timer in `QueryDNSSeeds` was introduced recently, I hadn't really thought about this during the last rebase.
I'll add this to the cleanup commit after the move.

📝 BenWestgate opened a pull request: "doc: Change GiB to GB in release-process.md"
(https://github.com/bitcoin/bitcoin/pull/30171)
This will make it consistent with the Welcome GUI and code comments which use "GB" for the `m_assumed_blockchain_size` and `m_assumed_chain_state_size` values.

Since they are used as GB values to calculate disk space in GB needed in the GUI, measuring size in GiB here will cause users to run out of space without a graphical warning. The error between units is over 45 GB for the full chain.

This doc is the only place we say they are GiB, and while an 8-10% overhead can account for the unit
...

💬 nsvrn commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2130009520)
Not sure if useful in this context but RocksDB also has some option which separates the values from keys into a separate log file, which is apparently useful in case values are very large in comparison to the keys and an optimization that works better for SSD drives.
https://rocksdb.org/blog/2021/05/26/integrated-blob-db.html

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2130012490)
> I feel like I'm missing something here though. Before the changes of this pull, the fixed seeds would be pulled after a minute of trying DNS seeds and not reaching the desired peer goal. However, after this pull `ProcessFixedSeeds` is called sequentially after `QueryDNSSeeds`. And the latter has no exit logic based on how long that has been running, meaning that if both are set `ProcessFixedSeeds` will never trigger (?).
>
> PS: Well, technically that's not completely accurate. This will fi
...

💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785731)
Done.

💬 sipa commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613785818)
Done.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1613785831)
> The spec recommend trying again with ever shorter intervals, but no less than 4 seconds, as the deadline approached. But it seems fine to not bother.

Right, the retrying is not according to spec-because i had the same thought. It seems overkill to adaptive timing for something that's meant to interact with the local router. If it doesn't have enough capacity to handle a PCP/NAT-PMP packet it sure won't be routing a lot of traffic. i was more afraid that bitcoind (or the system it's running
...