💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2129605456)
rebased on master :rocket:

👍 instagibbs approved a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2074162009)
utACK ef8de26be5478729328ac9d8a9ad6898351552b6

💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1611810616)
non-blocking nit: commit message still referencing TxRequest

💬 theuni commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1613558242)
`.Subset()` and `.Superset()`?

Without these comments, I would have to guess at the meanings (and I would have guessed wrong).

💬 maflcko commented on issue "Restore wallet taking forever to load":
(https://github.com/bitcoin/bitcoin/issues/30108#issuecomment-2129698658)
Any update on this?

💬 jonatack commented on issue "Apparently CJDNS network does not work with Tor on mainnet.":
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2129729113)
Network redundancy is valuable. We’ve seen attacks on Tor and I2P -- the latter very recently. If both Tor and I2P are taken down the same time, CJDNS use may increase quickly.

I think CJDNS mainly needs more awareness, an easy tutorial, and integration by node-in-box packages (see https://x.com/jonatack/status/1794016131272814708 that I just posted, suggesting bounties for that). These actions made a large difference in I2P use by bitcoin nodes.

💬 maflcko commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613608895)
```suggestion
size_t FirstPart() const noexcept { return std::min(m_capacity - m_offset, m_size); }
```

style-nit: I think `inline` can be dropped, according to https://en.cppreference.com/w/cpp/language/inline

> A function defined entirely inside a [class/struct/union definition](https://en.cppreference.com/w/cpp/language/classes), whether it's a member function or a non-member friend function, is implicitly an inline function [...]

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613613375)
I'm happy to rename `nFD`, but I kept it because it is not locally defined, but defined in the namespace, so at least one other method is using it. If we don't mind a bigger diff I'm happy to change it

👍 alfonsoromanz approved a pull request: "Bugfix - don't allow multiple dialogs for same tx in TransactionView"
(https://github.com/bitcoin-core/gui/pull/817#pullrequestreview-2077176859)
Re ACK ba13f10f9ca5bad7de0e140b667fbae5e8b7b9a3. I tested in Mac and now the dialog is brought to the foreground

https://github.com/bitcoin-core/gui/assets/19962151/dd21afaa-b48d-4082-be4c-3c3bdc965cfa

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613630608)
I agree the if check should be updated, but I don't necessarily agree with splitting `MAX_ADDNODE_CONNECTIONS` from `min_required_fds`.

There is the outstanding question of whether the minimum required fds to run Core should account for some of the connections. Currently, we are not accounting for any (given the check is performed against `MIN_CORE_FDS`), but I think we should be accounting for at least, outbound + manuals

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613633416)
I think that makes more sense

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2129822038)
Thanks for the suggestions @vasild, I've partially covered them and commented on some of the pending ones

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

`inline` does have an effect beyond making it an inline function (in GCC and Clang it increases the eagerness of the compiler to actually inline the function), but that's the sort of optimization one should only do guided by benchmarks, which I haven't done, so I dropped the `inline` here.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1613674776)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1613675392)
Replaced with `Overlaps()`.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1613675918)
Replaced with `IsSubsetOf()` and `IsSupersetOf()`.

💬 theuni commented on pull request "fuzz: Fix wallet_bdb_parser stdlib error matching":
(https://github.com/bitcoin/bitcoin/pull/30169#issuecomment-2129867229)
utACK.

Was this caught by review or did you run into it? Just curious if there are others.

💬 achow101 commented on pull request "fuzz: Fix wallet_bdb_parser stdlib error matching":
(https://github.com/bitcoin/bitcoin/pull/30169#issuecomment-2129882575)
ACK fac72985292b516919a216d9a78cf84418cd7f96

💬 jonatack commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2129888738)
Thanks @lcharles123 for the info.

> "Local addresses" section of "netinfo" does not show the IPv6 addresses added using "externalip" config option. (Probably there is no announce of this addresses).

CLI `-netinfo` will only return the local addresses that are returned by RPC `getnetworkinfo`, as under the hood, -netinfo calls getnetworkinfo to fetch that info.

Of the networks getnetworkinfo returns to you, IPv6 (and CJDNS, which is an IPv6 overlay network) are not reachable, which is wh
...

💬 glozow commented on pull request "util: add RingBuffer":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1613663028)
`static_assert(!std::is_trivially_copyable_v<TrackedObj<1>>)` ?