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

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

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

💬 theStack commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613786034)
In Rust they call it [`VecDeque`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html), maybe that's a naming option? (It even rhymes!)

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

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

💬 BenWestgate commented on issue "Welcome screen uses GiB values as GB, risking running out of space":
(https://github.com/bitcoin-core/gui/issues/821#issuecomment-2130014340)
On further analysis, the code comments in chainparams.h also think these values should be GB so I understand the origin of the gui error. It seems the release-process.md is the odd one out of the 3. This issue can be closed if the doc: PR below is ACK'd.
https://github.com/bitcoin/bitcoin/pull/30171

💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2130020267)
[6430bea ](https://github.com/bitcoin/bitcoin/commit/6430bea749d2c5832faeae12e69fe078680e01d3)to [c97bd16](https://github.com/bitcoin/bitcoin/commit/c97bd161212d56c167d3552da3b13db5ab2bc75e):
Addressed feedback by @sr-gi, thanks!

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2130021361)
1cbbcba42cf1c050022a73cb02b9d385fa184f2e: squashed all fixups to clean up the list of commits, no other changes

🚀 achow101 merged a pull request: "fuzz: Fix wallet_bdb_parser stdlib error matching"
(https://github.com/bitcoin/bitcoin/pull/30169)

📝 achow101 opened a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172)
Adds error messages that were not being handled. Also removes error messages that no longer exist.

Fixes #30166

💬 epiccurious commented on pull request "doc: Change GiB to GB in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130032118)
Concept ACK c04edce83db41cfce7a0a2f3418cf830c2e8afb5.

This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?

💬 epiccurious commented on pull request "refactor: Use type-safe time in txorphanage":
(https://github.com/bitcoin/bitcoin/pull/30170#issuecomment-2130040435)
utACK fa6d4891c7815025ab1cd58d0906466af31bb648.

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

💬 BenWestgate commented on pull request "doc: Change GiB to GB in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/30171#issuecomment-2130058903)
> Concept ACK [c04edce](https://github.com/bitcoin/bitcoin/commit/c04edce83db41cfce7a0a2f3418cf830c2e8afb5).
>
> This assumes the value should be displayed in GB. Is there a benefit to doing the calculation and displaying in GiB instead?

Software usually displays large sizes in GB because that's what the operating system and storage manufacturers use.

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613803860)
My comment is just about the commit message for this commit, not this line of code (sorry):

> Relax assumptions aabout in-mempool children of in-mempool

"aabout" --> "about"

> TxA (in mempool) <- TxB' (in package, conflicts with TxB) <-
> TxD (in package)

s/TxD/TxC/ ?

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613810585)
Seems weird to loop through all the workspaces, when `GetEntriesForConflicts` is doing all the work that needs to be done on the first invocation? Maybe just invoke once with the child's txid, as is done further down?

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613820116)
s/V3 transactions/TRUC transactions/, perhaps?

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1613817141)
Can you just add a comment here that references the comment on line 1520, as a reminder that we can't relax this without revisiting how we track available inputs when processing a package?